diff --git a/interfaces/builtin/greengrass_support.go b/interfaces/builtin/greengrass_support.go index 25558019093..524b6922302 100644 --- a/interfaces/builtin/greengrass_support.go +++ b/interfaces/builtin/greengrass_support.go @@ -22,6 +22,8 @@ package builtin import ( "github.com/snapcore/snapd/interfaces" "github.com/snapcore/snapd/interfaces/apparmor" + "github.com/snapcore/snapd/interfaces/seccomp" + "github.com/snapcore/snapd/interfaces/udev" "github.com/snapcore/snapd/release" ) @@ -51,7 +53,18 @@ const greengrassSupportConnectedPlugAppArmorCore = ` /system-data/var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/*/ggc-writable/{,**} rw, ` -const greengrassSupportConnectedPlugAppArmor = ` +const greengrassSupportProcessModeConnectedPlugAppArmor = ` +# Description: can manage greengrass 'things' and their sandboxes. This policy +# is meant currently only to enable Greengrass to run _only_ process-mode or +# "no container" lambdas. +# needed by older versions of cloneBinary.ensureSelfCloned() to avoid +# CVE-2019-5736 +/ ix, +# newer versions of runC have this denial instead of "/ ix" above +/bin/runc rix, +` + +const greengrassSupportFullContainerConnectedPlugAppArmor = ` # Description: can manage greengrass 'things' and their sandboxes. This # policy is intentionally not restrictive and is here to help guard against # programming errors and not for security confinement. The greengrassd @@ -380,13 +393,52 @@ mknodat - - |S_IFCHR - ` func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { - if release.OnClassic { - spec.AddSnippet(greengrassSupportConnectedPlugAppArmor) - } else { - spec.AddSnippet(greengrassSupportConnectedPlugAppArmor + greengrassSupportConnectedPlugAppArmorCore) + // check the flavor + var flavor string + _ = plug.Attr("flavor", &flavor) + switch flavor { + case "", "legacy-container": + // default, legacy version of the interface + if release.OnClassic { + spec.AddSnippet(greengrassSupportFullContainerConnectedPlugAppArmor) + } else { + spec.AddSnippet(greengrassSupportFullContainerConnectedPlugAppArmor + greengrassSupportConnectedPlugAppArmorCore) + } + // greengrass needs to use ptrace for controlling it's containers + spec.SetUsesPtraceTrace() + case "no-container": + // this is the no-container version, it does not use as much privilege + // as the default "legacy-container" flavor + spec.AddSnippet(greengrassSupportProcessModeConnectedPlugAppArmor) + } + + return nil +} + +func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { + // check the flavor + var flavor string + _ = plug.Attr("flavor", &flavor) + switch flavor { + case "", "legacy-container": + spec.AddSnippet(greengrassSupportConnectedPlugSeccomp) + case "no-container": + // no-container has no additional seccomp available to it + } + + return nil +} + +func (iface *greengrassSupportInterface) UDevConnectedPlug(spec *udev.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { + var flavor string + _ = plug.Attr("flavor", &flavor) + switch flavor { + case "", "legacy-container": + // default containerization controls the device cgroup + spec.SetControlsDeviceCgroup() + case "no-container": + // no-container does not control the device cgroup } - // greengrass needs to use ptrace - spec.SetUsesPtraceTrace() return nil } @@ -404,7 +456,5 @@ func init() { implicitOnClassic: true, baseDeclarationSlots: greengrassSupportBaseDeclarationSlots, baseDeclarationPlugs: greengrassSupportBaseDeclarationPlugs, - connectedPlugSecComp: greengrassSupportConnectedPlugSeccomp, - controlsDeviceCgroup: true, }}) } diff --git a/interfaces/builtin/greengrass_support_test.go b/interfaces/builtin/greengrass_support_test.go index 38183ac1735..4f0057ab2e6 100644 --- a/interfaces/builtin/greengrass_support_test.go +++ b/interfaces/builtin/greengrass_support_test.go @@ -42,6 +42,14 @@ type GreengrassSupportInterfaceSuite struct { extraSlot *interfaces.ConnectedSlot extraPlugInfo *snap.PlugInfo extraPlug *interfaces.ConnectedPlug + + // for the process flavor + processModePlugInfo *snap.PlugInfo + processModePlug *interfaces.ConnectedPlug + + // for the container flavor + containerModePlugInfo *snap.PlugInfo + containerModePlug *interfaces.ConnectedPlug } const coreSlotYaml = `name: core @@ -53,10 +61,26 @@ slots: ` const ggMockPlugSnapInfoYaml = `name: other version: 1.0 +plugs: + greengrass-support-legacy-container: + interface: greengrass-support + flavor: legacy-container apps: app2: command: foo - plugs: [greengrass-support, network-control] + plugs: [greengrass-support-legacy-container, greengrass-support, network-control] +` + +const ggProcessModeMockPlugSnapInfoYaml = `name: other +version: 1.0 +plugs: + greengrass-support-no-container: + interface: greengrass-support + flavor: no-container +apps: + app2: + command: foo + plugs: [greengrass-support-no-container, network-control] ` var _ = Suite(&GreengrassSupportInterfaceSuite{ @@ -69,6 +93,10 @@ func (s *GreengrassSupportInterfaceSuite) SetUpTest(c *C) { s.extraPlug, s.extraPlugInfo = MockConnectedPlug(c, ggMockPlugSnapInfoYaml, nil, "network-control") s.extraSlot, s.extraSlotInfo = MockConnectedSlot(c, coreSlotYaml, nil, "network-control") + s.processModePlug, s.processModePlugInfo = MockConnectedPlug(c, ggProcessModeMockPlugSnapInfoYaml, nil, "greengrass-support-no-container") + + s.containerModePlug, s.containerModePlugInfo = MockConnectedPlug(c, ggMockPlugSnapInfoYaml, nil, "greengrass-support-legacy-container") + } func (s *GreengrassSupportInterfaceSuite) TestName(c *C) { @@ -84,39 +112,86 @@ func (s *GreengrassSupportInterfaceSuite) TestSanitizePlug(c *C) { } func (s *GreengrassSupportInterfaceSuite) TestAppArmorSpec(c *C) { + + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + spec := &apparmor.Specification{} + c.Assert(spec.AddConnectedPlug(s.iface, plug, s.slot), IsNil) + c.Assert(spec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) + c.Check(spec.SnippetForTag("snap.other.app2"), testutil.Contains, "mount options=(rw, bind) /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** -> /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** ,\n") + c.Check(spec.UsesPtraceTrace(), Equals, true) + } +} + +func (s *GreengrassSupportInterfaceSuite) TestProcessModeAppArmorSpec(c *C) { spec := &apparmor.Specification{} - c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil) + c.Assert(spec.AddConnectedPlug(s.iface, s.processModePlug, s.slot), IsNil) c.Assert(spec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) - c.Check(spec.SnippetForTag("snap.other.app2"), testutil.Contains, "mount options=(rw, bind) /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** -> /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** ,\n") - c.Check(spec.UsesPtraceTrace(), Equals, true) + c.Check(spec.SnippetForTag("snap.other.app2"), testutil.Contains, "/ ix,\n") + c.Check(spec.SnippetForTag("snap.other.app2"), Not(testutil.Contains), "mount options=(rw, bind) /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** -> /var/snap/{@{SNAP_NAME},@{SNAP_INSTANCE_NAME}}/** ,\n") + c.Check(spec.UsesPtraceTrace(), Equals, false) } func (s *GreengrassSupportInterfaceSuite) TestSecCompSpec(c *C) { + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + spec := &seccomp.Specification{} + c.Assert(spec.AddConnectedPlug(s.iface, plug, s.slot), IsNil) + c.Check(spec.SnippetForTag("snap.other.app2"), testutil.Contains, "# for overlayfs and various bind mounts\nmount\numount2\npivot_root\n") + } +} + +func (s *GreengrassSupportInterfaceSuite) TestProcessModeSecCompSpec(c *C) { spec := &seccomp.Specification{} - c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil) - c.Check(spec.SnippetForTag("snap.other.app2"), testutil.Contains, "# for overlayfs and various bind mounts\nmount\numount2\npivot_root\n") + c.Assert(spec.AddConnectedPlug(s.iface, s.processModePlug, s.slot), IsNil) + c.Check(spec.SnippetForTag("snap.other.app2"), Not(testutil.Contains), "# for overlayfs and various bind mounts\nmount\numount2\npivot_root\n") } func (s *GreengrassSupportInterfaceSuite) TestUdevTaggingDisablingRemoveLast(c *C) { - // make a spec with network-control that has udev tagging - spec := &udev.Specification{} - c.Assert(spec.AddConnectedPlug(builtin.MustInterface("network-control"), s.extraPlug, s.extraSlot), IsNil) - c.Assert(spec.Snippets(), HasLen, 3) - - // connect the greengrass-support interface and ensure the spec is now nil - c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil) - c.Check(spec.Snippets(), HasLen, 0) + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + // make a spec with network-control that has udev tagging + spec := &udev.Specification{} + c.Assert(spec.AddConnectedPlug(builtin.MustInterface("network-control"), s.extraPlug, s.extraSlot), IsNil) + c.Assert(spec.Snippets(), HasLen, 3) + + // connect the greengrass-support interface and ensure the spec is now nil + c.Assert(spec.AddConnectedPlug(s.iface, plug, s.slot), IsNil) + c.Check(spec.Snippets(), HasLen, 0) + } } -func (s *GreengrassSupportInterfaceSuite) TestUdevTaggingDisablingRemoveFirst(c *C) { +func (s *GreengrassSupportInterfaceSuite) TestProcessModeUdevTaggingWorks(c *C) { spec := &udev.Specification{} // connect the greengrass-support interface and ensure the spec is nil - c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil) + c.Assert(spec.AddConnectedPlug(s.iface, s.processModePlug, s.slot), IsNil) c.Check(spec.Snippets(), HasLen, 0) - // add network-control and ensure the spec is still nil + // add network-control and now the spec is not nil c.Assert(spec.AddConnectedPlug(builtin.MustInterface("network-control"), s.extraPlug, s.extraSlot), IsNil) - c.Assert(spec.Snippets(), HasLen, 0) + c.Assert(spec.Snippets(), Not(HasLen), 0) +} + +func (s *GreengrassSupportInterfaceSuite) TestUdevTaggingDisablingRemoveFirst(c *C) { + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + spec := &udev.Specification{} + // connect the greengrass-support interface and ensure the spec is nil + c.Assert(spec.AddConnectedPlug(s.iface, s.plug, s.slot), IsNil) + c.Check(spec.Snippets(), HasLen, 0) + + // add network-control and ensure the spec is still nil + c.Assert(spec.AddConnectedPlug(builtin.MustInterface("network-control"), plug, s.extraSlot), IsNil) + c.Assert(spec.Snippets(), HasLen, 0) + } } func (s *GreengrassSupportInterfaceSuite) TestInterfaces(c *C) { @@ -127,24 +202,34 @@ func (s *GreengrassSupportInterfaceSuite) TestPermanentSlotAppArmorSessionNative restore := release.MockOnClassic(false) defer restore() - apparmorSpec := &apparmor.Specification{} - err := apparmorSpec.AddConnectedPlug(s.iface, s.plug, s.slot) - c.Assert(err, IsNil) - c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) - - // verify core rule present - c.Check(apparmorSpec.SnippetForTag("snap.other.app2"), testutil.Contains, "# /system-data/var/snap/greengrass/x1/ggc-writable/packages/1.7.0/var/worker/overlays/$UUID/upper/\n") + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + apparmorSpec := &apparmor.Specification{} + err := apparmorSpec.AddConnectedPlug(s.iface, plug, s.slot) + c.Assert(err, IsNil) + c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) + + // verify core rule present + c.Check(apparmorSpec.SnippetForTag("snap.other.app2"), testutil.Contains, "# /system-data/var/snap/greengrass/x1/ggc-writable/packages/1.7.0/var/worker/overlays/$UUID/upper/\n") + } } func (s *GreengrassSupportInterfaceSuite) TestPermanentSlotAppArmorSessionClassic(c *C) { restore := release.MockOnClassic(true) defer restore() - apparmorSpec := &apparmor.Specification{} - err := apparmorSpec.AddConnectedPlug(s.iface, s.plug, s.slot) - c.Assert(err, IsNil) - c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) - - // verify core rule not present - c.Check(apparmorSpec.SnippetForTag("snap.other.app2"), Not(testutil.Contains), "# /system-data/var/snap/greengrass/x1/ggc-writable/packages/1.7.0/var/worker/overlays/$UUID/upper/\n") + for _, plug := range []*interfaces.ConnectedPlug{ + s.plug, + s.containerModePlug, + } { + apparmorSpec := &apparmor.Specification{} + err := apparmorSpec.AddConnectedPlug(s.iface, plug, s.slot) + c.Assert(err, IsNil) + c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.other.app2"}) + + // verify core rule not present + c.Check(apparmorSpec.SnippetForTag("snap.other.app2"), Not(testutil.Contains), "# /system-data/var/snap/greengrass/x1/ggc-writable/packages/1.7.0/var/worker/overlays/$UUID/upper/\n") + } } diff --git a/interfaces/builtin/system_observe.go b/interfaces/builtin/system_observe.go index da1c3ce06ae..316044deb84 100644 --- a/interfaces/builtin/system_observe.go +++ b/interfaces/builtin/system_observe.go @@ -117,6 +117,13 @@ dbus (send) interface=org.freedesktop.DBus.Peer member=GetMachineId peer=(label=unconfined), + +# Allow reading if protected hardlinks are enabled, but don't allow enabling or +# disabling them +@{PROC}/sys/fs/protected_hardlinks r, +@{PROC}/sys/fs/protected_symlinks r, +@{PROC}/sys/fs/protected_fifos r, +@{PROC}/sys/fs/protected_regular r, ` const systemObserveConnectedPlugSecComp = ` diff --git a/run-checks b/run-checks index 5a6e67243a4..06082106e49 100755 --- a/run-checks +++ b/run-checks @@ -241,12 +241,14 @@ if [ "$STATIC" = 1 ]; then git ls-files -z -- . ':!:./po' ':!:./vendor' | xargs -0 misspell -error -i "$MISSPELL_IGNORE" - echo "Checking for ineffective assignments" - if ! command -v ineffassign >/dev/null; then - go get -u github.com/gordonklaus/ineffassign + if dpkg --compare-versions "$(go version | awk '$3 ~ /^go[0-9]/ {print substr($3, 3)}')" ge 1.12; then + echo "Checking for ineffective assignments" + if ! command -v ineffassign >/dev/null; then + go get -u github.com/gordonklaus/ineffassign + fi + # ineffassign knows about ignoring vendor/ \o/ + ineffassign ./... fi - # ineffassign knows about ignoring vendor/ \o/ - ineffassign . echo "Checking for naked returns" if ! command -v nakedret >/dev/null; then diff --git a/tests/lib/snaps/test-snapd-sh/bin/cmd b/tests/lib/snaps/test-snapd-sh/bin/cmd new file mode 100755 index 00000000000..e55f49a5a09 --- /dev/null +++ b/tests/lib/snaps/test-snapd-sh/bin/cmd @@ -0,0 +1,6 @@ +#!/bin/sh +PS1='$ ' +command="$1" +shift + +exec "$command" "$@" diff --git a/tests/lib/snaps/test-snapd-sh/meta/snap.yaml b/tests/lib/snaps/test-snapd-sh/meta/snap.yaml index 1081e8f8bb5..27ea1dc5fe7 100644 --- a/tests/lib/snaps/test-snapd-sh/meta/snap.yaml +++ b/tests/lib/snaps/test-snapd-sh/meta/snap.yaml @@ -5,3 +5,5 @@ version: 1.0 apps: sh: command: bin/sh + cmd: + command: bin/cmd diff --git a/tests/main/cohorts/task.yaml b/tests/main/cohorts/task.yaml index 49af1a1d972..a3bfe4734cb 100644 --- a/tests/main/cohorts/task.yaml +++ b/tests/main/cohorts/task.yaml @@ -1,13 +1,19 @@ summary: Check that cohorts work prepare: | - snap install yq - snap connect yq:home + "$TESTSTOOLS"/snaps-state install-local test-snapd-sh + +debug: | + cat coh.yml || true execute: | echo "Test we can create chorts:" snap create-cohort test-snapd-tools > coh.yml - COHORT=$( yq r coh.yml cohorts.test-snapd-tools.cohort-key ) + # the YAML looks like this: + # cohorts: + # test-snapd-tools: + # cohort-key: + COHORT=$(test-snapd-sh.cmd python3 -c 'import sys, yaml; print(yaml.safe_load(sys.stdin)["cohorts"]["test-snapd-tools"]["cohort-key"])' < coh.yml) test -n "$COHORT" echo "Test we can install from there:"