interface/screen_inhibit_control: Improve screen inhibit control for use on core

interfaces/builtin/screen_inhibit_control.go

@@ -19,13 +19,27 @@
 
 package builtin
 
+import (
+	"strings"
+
+	"github.com/snapcore/snapd/interfaces"
+	"github.com/snapcore/snapd/interfaces/apparmor"
+)
+
 const screenInhibitControlSummary = `allows inhibiting the screen saver`
 
 const screenInhibitBaseDeclarationSlots = `
   screen-inhibit-control:
     allow-installation:
       slot-snap-type:
+        - app
         - core
+    deny-auto-connection:
+      slot-snap-type:
+        - app
+    deny-connection:
+      slot-snap-type:
+        - app
 `
 
 const screenInhibitControlConnectedPlugAppArmor = `
@@ -39,21 +53,21 @@
     path=/org/gnome/SessionManager
     interface=org.gnome.SessionManager
     member={Inhibit,Uninhibit}
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
 
 # unity screen API
 dbus (send)
     bus=system
     interface="org.freedesktop.DBus.Introspectable"
     path="/com/canonical/Unity/Screen"
     member="Introspect"
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
 dbus (send)
     bus=system
     interface="com.canonical.Unity.Screen"
     path="/com/canonical/Unity/Screen"
     member={keepDisplayOn,removeDisplayOnRequest}
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
 
 # freedesktop.org ScreenSaver
 # compatibility rule
@@ -62,7 +76,7 @@
     path=/Screensaver
     interface=org.freedesktop.ScreenSaver
     member={Inhibit,UnInhibit,SimulateUserActivity}
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
 
 # xfce4-power-manager -
 # https://github.com/xfce-mirror/xfce4-power-manager/blob/0b3ad06ad4f51eae1aea3cdc26f434d8b5ce763e/src/org.freedesktop.PowerManagement.Inhibit.xml
@@ -71,31 +85,133 @@
     path=/org/freedesktop/PowerManagement/Inhibit
     interface=org.freedesktop.PowerManagement.Inhibit
     member={Inhibit,UnInhibit}
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
+dbus (receive)
+    bus=session
+    path=/org/freedesktop/PowerManagement/Inhibit
+    interface=org.freedesktop.PowerManagement.Inhibit
+    member=HasInhibitChanged
+    peer=(label=###SLOT_SECURITY_TAGS###),
 
 # API rule
 dbus (send)
     bus=session
     path=/{,org/freedesktop/,org/gnome/}ScreenSaver
     interface=org.{freedesktop,gnome}.ScreenSaver
     member={Inhibit,UnInhibit,SimulateUserActivity}
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
 
 # gnome, kde and cinnamon screensaver
 dbus (send)
     bus=session
     path=/{,ScreenSaver}
     interface=org.{gnome.ScreenSaver,kde.screensaver,cinnamon.ScreenSaver}
     member=SimulateUserActivity
-    peer=(label=unconfined),
+    peer=(label=###SLOT_SECURITY_TAGS###),
+`
+
+const screenInhibitControlConnectedSlotAppArmor = `
+# Description: Can inhibit and uninhibit screen savers in desktop sessions.
+#include <abstractions/dbus-session-strict>
+#include <abstractions/dbus-strict>
+
+# gnome-session
+dbus (receive)
+    bus=session
+    path=/org/gnome/SessionManager
+    interface=org.gnome.SessionManager
+    member={Inhibit,Uninhibit}
+    peer=(label=###PLUG_SECURITY_TAGS###),
+
+# unity screen API
+dbus (receive)
+    bus=system
+    interface="org.freedesktop.DBus.Introspectable"
+    path="/com/canonical/Unity/Screen"
+    member="Introspect"
+    peer=(label=###PLUG_SECURITY_TAGS###),
+dbus (receive)
+    bus=system
+    interface="com.canonical.Unity.Screen"
+    path="/com/canonical/Unity/Screen"
+    member={keepDisplayOn,removeDisplayOnRequest}
+    peer=(label=###PLUG_SECURITY_TAGS###),
+
+# freedesktop.org ScreenSaver
+# compatibility rule
+dbus (receive)
+    bus=session
+    path=/Screensaver
+    interface=org.freedesktop.ScreenSaver
+    member={Inhibit,UnInhibit,SimulateUserActivity}
+    peer=(label=###PLUG_SECURITY_TAGS###),
+
+# xfce4-power-manager -
+# https://github.com/xfce-mirror/xfce4-power-manager/blob/0b3ad06ad4f51eae1aea3cdc26f434d8b5ce763e/src/org.freedesktop.PowerManagement.Inhibit.xml
+dbus (receive)
+    bus=session
+    path=/org/freedesktop/PowerManagement/Inhibit
+    interface=org.freedesktop.PowerManagement.Inhibit
+    member={Inhibit,UnInhibit}
+    peer=(label=###PLUG_SECURITY_TAGS###),
+dbus (send)
+    bus=session
+    path=/org/freedesktop/PowerManagement/Inhibit
+    interface=org.freedesktop.PowerManagement.Inhibit
+    member=HasInhibitChanged
+    peer=(label=###PLUG_SECURITY_TAGS###),
+
+# API rule
+dbus (receive)
+    bus=session
+    path=/{,org/freedesktop/,org/gnome/}ScreenSaver
+    interface=org.{freedesktop,gnome}.ScreenSaver
+    member={Inhibit,UnInhibit,SimulateUserActivity}
+    peer=(label=###PLUG_SECURITY_TAGS###),
+
+# gnome, kde and cinnamon screensaver
+dbus (receive)
+    bus=session
+    path=/{,ScreenSaver}
+    interface=org.{gnome.ScreenSaver,kde.screensaver,cinnamon.ScreenSaver}
+    member=SimulateUserActivity
+    peer=(label=###PLUG_SECURITY_TAGS###),
 `
 
+type screenInhibitControlInterface struct {
+	commonInterface
+}
+
+func (iface *screenInhibitControlInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error {
+	old := "###SLOT_SECURITY_TAGS###"
+	var new string
+	if implicitSystemConnectedSlot(slot) {
+		// we are running on a system that has the screen-inhibit-control slot
+		// provided by the OS snap and so will run unconfined
+		new = "unconfined"
+	} else {
+		new = slot.LabelExpression()
+	}
+	snippet := strings.Replace(screenInhibitControlConnectedPlugAppArmor, old, new, -1)
+	spec.AddSnippet(snippet)
+	return nil
+}
+
+func (iface *screenInhibitControlInterface) AppArmorConnectedSlot(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error {
+	old := "###PLUG_SECURITY_TAGS###"
+	var new = plug.LabelExpression()
+	snippet := strings.Replace(screenInhibitControlConnectedSlotAppArmor, old, new, -1)
+	spec.AddSnippet(snippet)
+	return nil
+}
+
 func init() {
-	registerIface(&commonInterface{
-		name:                  "screen-inhibit-control",
-		summary:               screenInhibitControlSummary,
-		implicitOnClassic:     true,
-		baseDeclarationSlots:  screenInhibitBaseDeclarationSlots,
-		connectedPlugAppArmor: screenInhibitControlConnectedPlugAppArmor,
+	registerIface(&screenInhibitControlInterface{
+		commonInterface: commonInterface{
+			name:                 "screen-inhibit-control",
+			summary:              screenInhibitControlSummary,
+			implicitOnClassic:    true,
+			baseDeclarationSlots: screenInhibitBaseDeclarationSlots,
+		},
 	})
 }

interfaces/policy/basedeclaration_test.go

@@ -143,19 +143,20 @@ func (s *baseDeclSuite) TestAutoConnection(c *C) {
 	// these have more complex or in flux policies and have their
 	// own separate tests
 	snowflakes := map[string]bool{
-		"content":            true,
-		"core-support":       true,
-		"desktop":            true,
-		"home":               true,
-		"lxd-support":        true,
-		"microstack-support": true,
-		"multipass-support":  true,
-		"packagekit-control": true,
-		"pkcs11":             true,
-		"remoteproc":         true,
-		"snapd-control":      true,
-		"upower-observe":     true,
-		"empty":              true,
+		"content":                true,
+		"core-support":           true,
+		"desktop":                true,
+		"home":                   true,
+		"lxd-support":            true,
+		"microstack-support":     true,
+		"multipass-support":      true,
+		"packagekit-control":     true,
+		"pkcs11":                 true,
+		"remoteproc":             true,
+		"screen-inhibit-control": true,
+		"snapd-control":          true,
+		"upower-observe":         true,
+		"empty":                  true,
 	}
 
 	// these simply auto-connect, anything else doesn't
@@ -173,7 +174,6 @@ func (s *baseDeclSuite) TestAutoConnection(c *C) {
 		"opengl":                  true,
 		"optical-drive":           true,
 		"ros-opt-data":            true,
-		"screen-inhibit-control":  true,
 		"ubuntu-download-manager": true,
 		"unity7":                  true,
 		"unity8":                  true,
@@ -205,8 +205,9 @@ func (s *baseDeclSuite) TestAutoConnectionImplicitSlotOnly(c *C) {
 
 	// these auto-connect only with an implicit slot
 	autoconnect := map[string]bool{
-		"desktop":        true,
-		"upower-observe": true,
+		"desktop":                true,
+		"screen-inhibit-control": true,
+		"upower-observe":         true,
 	}
 
 	for _, iface := range all {
@@ -852,6 +853,7 @@ var (
 		"sd-control":                {"core"},
 		"serial-port":               {"core", "gadget"},
 		"spi":                       {"core", "gadget"},
+		"screen-inhibit-control":    {"core", "app"},
 		"steam-support":             {"core"},
 		"storage-framework-service": {"app"},
 		"thumbnailer-service":       {"app"},
@@ -1109,6 +1111,7 @@ func (s *baseDeclSuite) TestConnection(c *C) {
 		"posix-mq":                  true,
 		"qualcomm-ipc-router":       true,
 		"raw-volume":                true,
+		"screen-inhibit-control":    true,
 		"shared-memory":             true,
 		"storage-framework-service": true,
 		"thumbnailer-service":       true,
@@ -1139,9 +1142,10 @@ func (s *baseDeclSuite) TestConnectionImplicitSlotOnly(c *C) {
 
 	// these allow connect only with an implicit slot
 	autoconnect := map[string]bool{
-		"desktop":             true,
-		"qualcomm-ipc-router": true,
-		"upower-observe":      true,
+		"desktop":                true,
+		"qualcomm-ipc-router":    true,
+		"screen-inhibit-control": true,
+		"upower-observe":         true,
 	}
 
 	for _, iface := range all {
@@ -1334,6 +1338,7 @@ func (s *baseDeclSuite) TestValidity(c *C) {
 		"polkit-agent":                     true,
 		"remoteproc":                       true,
 		"qualcomm-ipc-router":              true,
+		"screen-inhibit-control":           true,
 		"sd-control":                       true,
 		"shutdown":                         true,
 		"shared-memory":                    true,
@@ -1884,3 +1889,35 @@ plugs:
 	err = ic.Check()
 	c.Assert(err, IsNil)
 }
+
+func (s *baseDeclSuite) TestConnectionScreenInhibitControl(c *C) {
+	cand := s.connectCand(c, "screen-inhibit-control", "", "")
+	err := cand.Check()
+	c.Assert(err, ErrorMatches, `connection denied by slot rule of interface "screen-inhibit-control"`)
+
+	plugsSlots := `
+plugs:
+  screen-inhibit-control:
+    allow-connection: true
+`
+	snapDecl := s.mockSnapDecl(c, "some-snap", "some-snap", "canonical", plugsSlots)
+	cand.PlugSnapDeclaration = snapDecl
+	err = cand.Check()
+	c.Assert(err, IsNil)
+}
+
+func (s *baseDeclSuite) TestAutoConnectionScreenInhibitControl(c *C) {
+	cand := s.connectCand(c, "screen-inhibit-control", "", "")
+	_, err := cand.CheckAutoConnect()
+	c.Assert(err, ErrorMatches, "auto-connection denied by slot rule of interface \"screen-inhibit-control\"")
+
+	plugsSlots := `
+plugs:
+  screen-inhibit-control:
+    allow-auto-connection: true
+`
+	snapDecl := s.mockSnapDecl(c, "some-snap", "some-snap", "canonical", plugsSlots)
+	cand.PlugSnapDeclaration = snapDecl
+	_, err = cand.CheckAutoConnect()
+	c.Check(err, IsNil)
+}