Merge pull request #9824 from anonymouse64/bugfix/2-48-backport-green…

…grass-flavors

interfaces/greengrass-support: back-port interface changes to 2.48

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

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

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 = `

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

tests/lib/snaps/test-snapd-sh/bin/cmd

@@ -0,0 +1,6 @@
+#!/bin/sh
+PS1='$ '
+command="$1"
+shift
+
+exec "$command" "$@"

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

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: <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:"