Merge pull request #9132 from jhenstridge/snapctl-is-connected-pid

o/hookstate/ctlcmd: add optional --pid and --apparmor-label arguments to "snapctl is-connected"

overlord/hookstate/ctlcmd/export_test.go

@@ -28,6 +28,11 @@ import (
 	"github.com/snapcore/snapd/snap"
 )
 
+const (
+	NotASnapCode    = notASnapCode
+	ClassicSnapCode = classicSnapCode
+)
+
 var AttributesTask = attributesTask
 
 func MockServicestateControlFunc(f func(*state.State, []*snap.AppInfo, *servicestate.Instruction, *servicestate.Flags, *hookstate.Context) ([]*state.TaskSet, error)) (restore func()) {
@@ -88,3 +93,11 @@ func (c *MockCommand) Execute(args []string) error {
 
 	return nil
 }
+
+func MockCgroupSnapNameFromPid(f func(int) (string, error)) (restore func()) {
+	old := cgroupSnapNameFromPid
+	cgroupSnapNameFromPid = f
+	return func() {
+		cgroupSnapNameFromPid = old
+	}
+}

overlord/hookstate/ctlcmd/is_connected.go

@@ -26,6 +26,16 @@ import (
 	"github.com/snapcore/snapd/interfaces"
 	"github.com/snapcore/snapd/overlord/ifacestate"
 	"github.com/snapcore/snapd/overlord/snapstate"
+	"github.com/snapcore/snapd/sandbox/apparmor"
+	"github.com/snapcore/snapd/sandbox/cgroup"
+	"github.com/snapcore/snapd/snap"
+)
+
+var cgroupSnapNameFromPid = cgroup.SnapNameFromPid
+
+const (
+	classicSnapCode = 10
+	notASnapCode    = 11
 )
 
 type isConnectedCommand struct {
@@ -34,6 +44,8 @@ type isConnectedCommand struct {
 	Positional struct {
 		PlugOrSlotSpec string `positional-args:"true" positional-arg-name:"<plug|slot>"`
 	} `positional-args:"true" required:"true"`
+	Pid           int    `long:"pid" description:"Process ID for a plausibly connected process"`
+	AppArmorLabel string `long:"apparmor-label" description:"AppArmor label for a plausibly connected process"`
 }
 
 var shortIsConnectedHelp = i18n.G(`Return success if the given plug or slot is connected, and failure otherwise`)
@@ -47,6 +59,15 @@ $ echo $?
 
 Snaps can only query their own plugs and slots - snap name is implicit and
 implied by the snapctl execution context.
+
+The --pid and --aparmor-label options can be used to determine whether
+a plug or slot is connected to the snap identified by the given
+process ID or AppArmor label.  In this mode, additional failure exit
+codes may be returned: 10 if the other snap is not connected but uses
+classic confinement, or 11 if the other process is not snap confined.
+
+The --pid and --apparmor-label options may only be used with slots of
+interface type "pulseaudio", "audio-record", or "cups-control".
 `)
 
 func init() {
@@ -55,6 +76,17 @@ func init() {
 	})
 }
 
+func isConnectedPidCheckAllowed(info *snap.Info, plugOrSlot string) bool {
+	slot := info.Slots[plugOrSlot]
+	if slot != nil {
+		switch slot.Interface {
+		case "pulseaudio", "audio-record", "cups-control":
+			return true
+		}
+	}
+	return false
+}
+
 func (c *isConnectedCommand) Execute(args []string) error {
 	plugOrSlot := c.Positional.PlugOrSlotSpec
 
@@ -87,6 +119,34 @@ func (c *isConnectedCommand) Execute(args []string) error {
 		return fmt.Errorf("internal error: cannot get connections: %s", err)
 	}
 
+	var otherSnap *snap.Info
+	if c.AppArmorLabel != "" {
+		if !isConnectedPidCheckAllowed(info, plugOrSlot) {
+			return fmt.Errorf("cannot use --apparmor-label check with %s:%s", snapName, plugOrSlot)
+		}
+		name, _, _, err := apparmor.DecodeLabel(c.AppArmorLabel)
+		if err != nil {
+			return &UnsuccessfulError{ExitCode: notASnapCode}
+		}
+		otherSnap, err = snapstate.CurrentInfo(st, name)
+		if err != nil {
+			return fmt.Errorf("internal error: cannot get snap info for AppArmor label %q: %s", c.AppArmorLabel, err)
+		}
+	} else if c.Pid != 0 {
+		if !isConnectedPidCheckAllowed(info, plugOrSlot) {
+			return fmt.Errorf("cannot use --pid check with %s:%s", snapName, plugOrSlot)
+		}
+		name, err := cgroupSnapNameFromPid(c.Pid)
+		if err != nil {
+			// Indicate that this pid is not a snap
+			return &UnsuccessfulError{ExitCode: notASnapCode}
+		}
+		otherSnap, err = snapstate.CurrentInfo(st, name)
+		if err != nil {
+			return fmt.Errorf("internal error: cannot get snap info for pid %d: %s", c.Pid, err)
+		}
+	}
+
 	// snapName is the name of the snap executing snapctl command, it's
 	// obtained from the context (ephemeral if run by apps, or full if run by
 	// hooks). plug and slot names are unique within a snap, so there is no
@@ -102,10 +162,20 @@ func (c *isConnectedCommand) Execute(args []string) error {
 
 		matchingPlug := connRef.PlugRef.Snap == snapName && connRef.PlugRef.Name == plugOrSlot
 		matchingSlot := connRef.SlotRef.Snap == snapName && connRef.SlotRef.Name == plugOrSlot
-		if matchingPlug || matchingSlot {
-			return nil
+		if otherSnap != nil {
+			if matchingPlug && connRef.SlotRef.Snap == otherSnap.InstanceName() || matchingSlot && connRef.PlugRef.Snap == otherSnap.InstanceName() {
+				return nil
+			}
+		} else {
+			if matchingPlug || matchingSlot {
+				return nil
+			}
 		}
 	}
 
+	if otherSnap != nil && otherSnap.Confinement == snap.ClassicConfinement {
+		return &UnsuccessfulError{ExitCode: classicSnapCode}
+	}
+
 	return &UnsuccessfulError{ExitCode: 1}
 }

overlord/hookstate/ctlcmd/is_connected_test.go

@@ -20,6 +20,8 @@
 package ctlcmd_test
 
 import (
+	"fmt"
+
 	"github.com/snapcore/snapd/dirs"
 	"github.com/snapcore/snapd/overlord/hookstate"
 	"github.com/snapcore/snapd/overlord/hookstate/ctlcmd"
@@ -74,6 +76,62 @@ var isConnectedTests = []struct {
 }, {
 	args: []string{"is-connected", "foo"},
 	err:  `snap "snap1" has no plug or slot named "foo"`,
+}, {
+	// snap1:plug1 does not use an allowed interface
+	args: []string{"is-connected", "--pid", "1002", "plug1"},
+	err:  `cannot use --pid check with snap1:plug1`,
+}, {
+	// snap1:slot1 does not use an allowed interface
+	args: []string{"is-connected", "--pid", "1002", "slot1"},
+	err:  `cannot use --pid check with snap1:slot1`,
+}, {
+	// snap1:cc slot is not connected to snap2
+	args:     []string{"is-connected", "--pid", "1002", "cc"},
+	exitCode: 1,
+}, {
+	// snap1:cc slot is connected to snap3
+	args:     []string{"is-connected", "--pid", "1003", "cc"},
+	exitCode: 0,
+}, {
+	// snap1:cc slot is not connected to a non-snap pid
+	args:     []string{"is-connected", "--pid", "42", "cc"},
+	exitCode: ctlcmd.NotASnapCode,
+}, {
+	// snap1:cc slot is connected to a classic snap5
+	args:     []string{"is-connected", "--pid", "1005", "cc"},
+	exitCode: 0,
+}, {
+	// snap1:audio-record slot is not connected to classic snap5
+	args:     []string{"is-connected", "--pid", "1005", "audio-record"},
+	exitCode: ctlcmd.ClassicSnapCode,
+}, {
+	// snap1:plug1 does not use an allowed interface
+	args: []string{"is-connected", "--apparmor-label", "snap.snap2.app", "plug1"},
+	err:  `cannot use --apparmor-label check with snap1:plug1`,
+}, {
+	// snap1:slot1 does not use an allowed interface
+	args: []string{"is-connected", "--apparmor-label", "snap.snap2.app", "slot1"},
+	err:  `cannot use --apparmor-label check with snap1:slot1`,
+}, {
+	// snap1:cc slot is not connected to snap2
+	args:     []string{"is-connected", "--apparmor-label", "snap.snap2.app", "cc"},
+	exitCode: 1,
+}, {
+	// snap1:cc slot is connected to snap3
+	args:     []string{"is-connected", "--apparmor-label", "snap.snap3.app", "cc"},
+	exitCode: 0,
+}, {
+	// snap1:cc slot is not connected to a non-snap pid
+	args:     []string{"is-connected", "--apparmor-label", "/usr/bin/evince", "cc"},
+	exitCode: ctlcmd.NotASnapCode,
+}, {
+	// snap1:cc slot is connected to a classic snap5
+	args:     []string{"is-connected", "--apparmor-label", "snap.snap5.app", "cc"},
+	exitCode: 0,
+}, {
+	// snap1:audio-record slot is not connected to classic snap5
+	args:     []string{"is-connected", "--apparmor-label", "snap.snap5.app", "audio-record"},
+	exitCode: ctlcmd.ClassicSnapCode,
 }}
 
 func mockInstalledSnap(c *C, st *state.State, snapYaml string) {
@@ -102,13 +160,50 @@ plugs:
     interface: x11
 slots:
   slot1:
+    interface: x11
+  cc:
+    interface: cups-control
+  audio-record:
+    interface: audio-record`)
+	mockInstalledSnap(c, s.st, `name: snap2
+slots:
+  slot2:
+    interface: x11`)
+	mockInstalledSnap(c, s.st, `name: snap3
+plugs:
+  plug4:
+    interface: x11
+  cc:
+    interface: cups-control
+slots:
+  slot3:
+    interface: x11`)
+	mockInstalledSnap(c, s.st, `name: snap4
+slots:
+  slot4:
     interface: x11`)
+	mockInstalledSnap(c, s.st, `name: snap5
+confinement: classic
+plugs:
+  cc:
+    interface: cups-control`)
+	restore := ctlcmd.MockCgroupSnapNameFromPid(func(pid int) (string, error) {
+		switch {
+		case 1000 < pid && pid < 1100:
+			return fmt.Sprintf("snap%d", pid-1000), nil
+		default:
+			return "", fmt.Errorf("Not a snap")
+		}
+	})
+	defer restore()
 
 	s.st.Set("conns", map[string]interface{}{
 		"snap1:plug1 snap2:slot2": map[string]interface{}{},
 		"snap1:plug2 snap3:slot3": map[string]interface{}{"undesired": true},
 		"snap1:plug3 snap4:slot4": map[string]interface{}{"hotplug-gone": true},
 		"snap3:plug4 snap1:slot1": map[string]interface{}{},
+		"snap3:cc snap1:cc":       map[string]interface{}{},
+		"snap5:cc snap1:cc":       map[string]interface{}{},
 	})
 
 	s.st.Unlock()

sandbox/apparmor/export_test.go

@@ -43,8 +43,6 @@ var (
 	RequiredParserFeatures  = requiredParserFeatures
 	PreferredKernelFeatures = preferredKernelFeatures
 	PreferredParserFeatures = preferredParserFeatures
-
-	DecodeLabel = decodeLabel
 )
 
 func FreshAppArmorAssessment() {

sandbox/apparmor/process.go

@@ -53,7 +53,7 @@ func labelFromPid(pid int) (string, error) {
 	return label, nil
 }
 
-func decodeLabel(label string) (snap, app, hook string, err error) {
+func DecodeLabel(label string) (snap, app, hook string, err error) {
 	parts := strings.Split(label, ".")
 	if parts[0] != "snap" {
 		return "", "", "", fmt.Errorf("security label %q does not belong to a snap", label)
@@ -72,5 +72,5 @@ func SnapAppFromPid(pid int) (snap, app, hook string, err error) {
 	if err != nil {
 		return "", "", "", err
 	}
-	return decodeLabel(label)
+	return DecodeLabel(label)
 }

tests/main/snapctl-is-connected-pid/task.yaml

@@ -0,0 +1,92 @@
+summary: Ensure "snapctl is-connected" --pid and --apparmor-label options work.
+
+prepare: |
+    tests.cleanup prepare
+    #shellcheck source=tests/lib/snaps.sh
+    . "$TESTSLIB"/snaps.sh
+    install_local test-snap1
+    install_local test-snap2
+
+    #shellcheck source=tests/lib/dirs.sh
+    . "$TESTSLIB"/dirs.sh
+    case "$SPREAD_SYSTEM" in
+        fedora-*|arch-*|centos-*)
+            # although classic snaps do not work out of the box on fedora,
+            # we still want to verify if the basics do work if the user
+            # symlinks /snap to $SNAP_MOUNT_DIR themselves
+            ln -sf $SNAP_MOUNT_DIR /snap
+            tests.cleanup defer rm -f /snap
+        ;;
+    esac
+
+restore: |
+    tests.cleanup restore
+
+execute: |
+    echo "The test-snap1 service is running"
+    systemctl is-active snap.test-snap1.svc.service
+    svc_pid=$(systemctl show --property=MainPID snap.test-snap1.svc.service | cut -d = -f 2)
+
+    expect_status() {
+        expected="$1"
+        shift
+        # Temporarily turn off "set -e" so we can check the exit status
+        set +e; "$@"; local ret=$?; set -e
+        test "$ret" -eq "$expected"
+    }
+
+    echo "Plugs and slots are initially disconnected"
+    not test-snap2.snapctl is-connected foo-slot
+
+    echo "Disconnected interfaces are not connected to a snap process"
+    expect_status 1 test-snap2.snapctl is-connected --pid "$svc_pid" foo-slot
+
+    echo "Disconnected interfaces are not connected to non-snap process"
+    expect_status 11 test-snap2.snapctl is-connected --pid 1 foo-slot
+
+    echo "Connect interface"
+    snap connect test-snap1:foo-plug test-snap2:foo-slot
+
+    echo "Connected interfaces report as connected to snap process"
+    test-snap2.snapctl is-connected --pid "$svc_pid" foo-slot
+
+    echo "Interfaces still not connected to non-snap process"
+    expect_status 11 test-snap2.snapctl is-connected --pid 1 foo-slot
+
+    if [[ "$(snap debug confinement)" = strict ]]; then
+      svc_label=$(sed 's/ (.*)$//' < "/proc/$svc_pid/attr/current")
+
+      echo "We can detect connected interfaces by AppArmor label too"
+      test-snap2.snapctl is-connected --apparmor-label "$svc_label" foo-slot
+      snap disconnect test-snap1:foo-plug test-snap2:foo-slot
+      expect_status 1 test-snap2.snapctl is-connected --apparmor-label "$svc_label" foo-slot
+
+      echo "Non-snap AppArmor labels return a special exit code"
+      expect_status 11 test-snap2.snapctl is-connected --apparmor-label /usr/bin/evince foo-slot
+    fi
+
+    # The remaining tests rely on classic confinement, so skip Ubuntu Core
+    if [[ "$SPREAD_SYSTEM" = ubuntu-core-* ]]; then
+        exit 0
+    fi
+    # We also skip Ubuntu 14.04, since it does not allow us track
+    # classic confined snap processes (there is no systemd based
+    # tracking, and they aren't added to a freezer cgroup).
+    if [[ "$SPREAD_SYSTEM" = ubuntu-14.04-* ]]; then
+        exit 0
+    fi
+
+    #shellcheck source=tests/lib/snaps.sh
+    . "$TESTSLIB"/snaps.sh
+    install_local_classic test-snap-classic
+
+    echo "The test-snap-classic service is running"
+    systemctl is-active snap.test-snap-classic.svc.service
+    classic_pid=$(systemctl show --property=MainPID snap.test-snap-classic.svc.service | cut -d = -f 2)
+
+    echo "Unconnected classic snaps report a special exit code"
+    expect_status 10 test-snap2.snapctl is-connected --pid "$classic_pid" foo-slot
+
+    echo "But still reports success when connected"
+    snap connect test-snap-classic:foo-plug test-snap2:foo-slot
+    test-snap2.snapctl is-connected --pid "$classic_pid" foo-slot

tests/main/snapctl-is-connected-pid/test-snap-classic/bin/service.sh

@@ -0,0 +1,4 @@
+#!/bin/sh
+
+echo "service running"
+exec sleep infinity