Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 16 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions overlord/hookstate/ctlcmd/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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
}
}
74 changes: 72 additions & 2 deletions overlord/hookstate/ctlcmd/is_connected.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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`)
Expand All @@ -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() {
Expand All @@ -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":
pedronis marked this conversation as resolved.
Show resolved Hide resolved
jhenstridge marked this conversation as resolved.
Show resolved Hide resolved
return true
}
}
return false
}

func (c *isConnectedCommand) Execute(args []string) error {
plugOrSlot := c.Positional.PlugOrSlotSpec

Expand Down Expand Up @@ -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
Expand All @@ -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}
}
95 changes: 95 additions & 0 deletions overlord/hookstate/ctlcmd/is_connected_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions sandbox/apparmor/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ var (
RequiredParserFeatures = requiredParserFeatures
PreferredKernelFeatures = preferredKernelFeatures
PreferredParserFeatures = preferredParserFeatures

DecodeLabel = decodeLabel
)

func FreshAppArmorAssessment() {
Expand Down
4 changes: 2 additions & 2 deletions sandbox/apparmor/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
92 changes: 92 additions & 0 deletions tests/main/snapctl-is-connected-pid/task.yaml
Original file line number Diff line number Diff line change
@@ -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
jhenstridge marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh

echo "service running"
exec sleep infinity
Loading