From 743cf5b40e4b7c176f324b037632e78531040bb2 Mon Sep 17 00:00:00 2001 From: Ian Johnson Date: Tue, 3 Nov 2020 16:09:17 -0600 Subject: [PATCH 1/9] interfaces/system-observe: add various accesses These accesses are generally useful to snaps wanting to know more about their host system, but also more specifically useful to the new version of the Greengrass snap which will use much less privilege to run. Signed-off-by: Ian Johnson --- interfaces/builtin/system_observe.go | 7 +++++++ 1 file changed, 7 insertions(+) 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 = ` From b0522c44963e5934f8bd29242eab01c08ed82be0 Mon Sep 17 00:00:00 2001 From: Ian Johnson Date: Tue, 3 Nov 2020 16:13:21 -0600 Subject: [PATCH 2/9] interfaces/greengrass-support: add additional "process" flavor for 1.11 update This adds a new attribute to the greengrass-support interface, "flavor", which indicates what mode of containerization the greengrassd daemon is meant to be supporting with the plug. With no flavor attribute, or the "container" flavor, then the old policy is available so as to not break old users of the snap, but with a new "process" flavor, then a far less privileged version of the interface is provided, which allows the greengrassd daemon to implement no containerization and thus the lambdas that are run are not run with the additional privilege afforded to the original implementation of the interface, as that would allow lambdas to trivially escape the sandbox. Signed-off-by: Ian Johnson --- interfaces/builtin/greengrass_support.go | 76 +++++++-- interfaces/builtin/greengrass_support_test.go | 149 ++++++++++++++---- 2 files changed, 184 insertions(+), 41 deletions(-) diff --git a/interfaces/builtin/greengrass_support.go b/interfaces/builtin/greengrass_support.go index 25558019093..fa0895ee41b 100644 --- a/interfaces/builtin/greengrass_support.go +++ b/interfaces/builtin/greengrass_support.go @@ -20,8 +20,12 @@ package builtin import ( + "fmt" + "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 +55,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 +395,58 @@ 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 "", "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 "process": + // this is the process-mode version, it does not use as much privilege + // as the default "container" flavor + spec.AddSnippet(greengrassSupportProcessModeConnectedPlugAppArmor) + default: + return fmt.Errorf("cannot add apparmor plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) + } + + 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 "", "container": + spec.AddSnippet(greengrassSupportConnectedPlugSeccomp) + case "process": + // process mode has no additional seccomp available to it + default: + return fmt.Errorf("cannot add seccomp plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) + } + + 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 "", "container": + // default containerization controls the device cgroup + spec.SetControlsDeviceCgroup() + case "process": + // process mode does not control the device cgroup + default: + return fmt.Errorf("cannot add udev plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) } - // greengrass needs to use ptrace - spec.SetUsesPtraceTrace() return nil } @@ -404,7 +464,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..215b3b44dee 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-container-mode: + interface: greengrass-support + flavor: container apps: app2: command: foo - plugs: [greengrass-support, network-control] + plugs: [greengrass-support-container-mode, greengrass-support, network-control] +` + +const ggProcessModeMockPlugSnapInfoYaml = `name: other +version: 1.0 +plugs: + greengrass-support-process-mode: + interface: greengrass-support + flavor: process +apps: + app2: + command: foo + plugs: [greengrass-support-process-mode, 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-process-mode") + + s.containerModePlug, s.containerModePlugInfo = MockConnectedPlug(c, ggMockPlugSnapInfoYaml, nil, "greengrass-support-container-mode") + } 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") + } } From 6d602b9d05552ec6599467a4f017daf32b4a0c46 Mon Sep 17 00:00:00 2001 From: Ian Johnson Date: Wed, 11 Nov 2020 07:54:26 -0600 Subject: [PATCH 3/9] interfaces/greengrass-support: don't fail if attribute is malformed Signed-off-by: Ian Johnson --- interfaces/builtin/greengrass_support.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/interfaces/builtin/greengrass_support.go b/interfaces/builtin/greengrass_support.go index fa0895ee41b..2e10dfa90a4 100644 --- a/interfaces/builtin/greengrass_support.go +++ b/interfaces/builtin/greengrass_support.go @@ -20,8 +20,6 @@ package builtin import ( - "fmt" - "github.com/snapcore/snapd/interfaces" "github.com/snapcore/snapd/interfaces/apparmor" "github.com/snapcore/snapd/interfaces/seccomp" @@ -412,8 +410,6 @@ func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Sp // this is the process-mode version, it does not use as much privilege // as the default "container" flavor spec.AddSnippet(greengrassSupportProcessModeConnectedPlugAppArmor) - default: - return fmt.Errorf("cannot add apparmor plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) } return nil @@ -428,8 +424,6 @@ func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Spec spec.AddSnippet(greengrassSupportConnectedPlugSeccomp) case "process": // process mode has no additional seccomp available to it - default: - return fmt.Errorf("cannot add seccomp plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) } return nil @@ -444,8 +438,6 @@ func (iface *greengrassSupportInterface) UDevConnectedPlug(spec *udev.Specificat spec.SetControlsDeviceCgroup() case "process": // process mode does not control the device cgroup - default: - return fmt.Errorf("cannot add udev plug policy: unsupported flavor attribute value %q for greengrass-support interface", flavor) } return nil From dfb839e198fa47139e0efbb45c7d2ccdf188c4fa Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 13 Nov 2020 10:12:50 +0100 Subject: [PATCH 4/9] interfaces: add XXX to greegras attr naming The PR that added greengras attribute names was merged prematurely before Samuele had a chance to review the naming. This commit adds the TODO and will be milestoned 2.49 to ensure that we do not release anything to stable with the preliminary names. --- interfaces/builtin/greengrass_support.go | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/interfaces/builtin/greengrass_support.go b/interfaces/builtin/greengrass_support.go index 2e10dfa90a4..a1ca35cb52c 100644 --- a/interfaces/builtin/greengrass_support.go +++ b/interfaces/builtin/greengrass_support.go @@ -392,12 +392,19 @@ mknod - |S_IFCHR - mknodat - - |S_IFCHR - ` +// XXX: decide on the final names and adjust tests too +const ( + flavorAttrStr = "flavor" + flavorContainer = "container" + falvorProcess = "process" +) + func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { // check the flavor var flavor string - _ = plug.Attr("flavor", &flavor) + _ = plug.Attr(flavorAttrStr, &flavor) switch flavor { - case "", "container": + case "", flavorContainer: // default, legacy version of the interface if release.OnClassic { spec.AddSnippet(greengrassSupportFullContainerConnectedPlugAppArmor) @@ -406,9 +413,9 @@ func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Sp } // greengrass needs to use ptrace for controlling it's containers spec.SetUsesPtraceTrace() - case "process": + case falvorProcess: // this is the process-mode version, it does not use as much privilege - // as the default "container" flavor + // as the default flavorContainer flavor spec.AddSnippet(greengrassSupportProcessModeConnectedPlugAppArmor) } @@ -418,11 +425,11 @@ func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Sp func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { // check the flavor var flavor string - _ = plug.Attr("flavor", &flavor) + _ = plug.Attr(flavorAttrStr, &flavor) switch flavor { - case "", "container": + case "", flavorContainer: spec.AddSnippet(greengrassSupportConnectedPlugSeccomp) - case "process": + case falvorProcess: // process mode has no additional seccomp available to it } @@ -431,12 +438,12 @@ func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Spec func (iface *greengrassSupportInterface) UDevConnectedPlug(spec *udev.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { var flavor string - _ = plug.Attr("flavor", &flavor) + _ = plug.Attr(flavorAttrStr, &flavor) switch flavor { - case "", "container": + case "", flavorContainer: // default containerization controls the device cgroup spec.SetControlsDeviceCgroup() - case "process": + case falvorProcess: // process mode does not control the device cgroup } From f3973d9b32c8607bd1f995533d6728c9ed2d47e3 Mon Sep 17 00:00:00 2001 From: Ian Johnson Date: Fri, 13 Nov 2020 10:06:57 -0600 Subject: [PATCH 5/9] interfaces/greengrass-support: finalize the flavor attribute names The flavor attribute names are now as follows: - "legacy-container" is the full containerization also known as "Greengrass container" in AWS's UI. - "no-container" is the process-mode, no confinement mode, also known as "no container" in AWS's UI. Signed-off-by: Ian Johnson --- interfaces/builtin/greengrass_support.go | 33 ++++++++----------- interfaces/builtin/greengrass_support_test.go | 16 ++++----- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/interfaces/builtin/greengrass_support.go b/interfaces/builtin/greengrass_support.go index a1ca35cb52c..524b6922302 100644 --- a/interfaces/builtin/greengrass_support.go +++ b/interfaces/builtin/greengrass_support.go @@ -392,19 +392,12 @@ mknod - |S_IFCHR - mknodat - - |S_IFCHR - ` -// XXX: decide on the final names and adjust tests too -const ( - flavorAttrStr = "flavor" - flavorContainer = "container" - falvorProcess = "process" -) - func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { // check the flavor var flavor string - _ = plug.Attr(flavorAttrStr, &flavor) + _ = plug.Attr("flavor", &flavor) switch flavor { - case "", flavorContainer: + case "", "legacy-container": // default, legacy version of the interface if release.OnClassic { spec.AddSnippet(greengrassSupportFullContainerConnectedPlugAppArmor) @@ -413,9 +406,9 @@ func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Sp } // greengrass needs to use ptrace for controlling it's containers spec.SetUsesPtraceTrace() - case falvorProcess: - // this is the process-mode version, it does not use as much privilege - // as the default flavorContainer flavor + 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) } @@ -425,12 +418,12 @@ func (iface *greengrassSupportInterface) AppArmorConnectedPlug(spec *apparmor.Sp func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { // check the flavor var flavor string - _ = plug.Attr(flavorAttrStr, &flavor) + _ = plug.Attr("flavor", &flavor) switch flavor { - case "", flavorContainer: + case "", "legacy-container": spec.AddSnippet(greengrassSupportConnectedPlugSeccomp) - case falvorProcess: - // process mode has no additional seccomp available to it + case "no-container": + // no-container has no additional seccomp available to it } return nil @@ -438,13 +431,13 @@ func (iface *greengrassSupportInterface) SecCompConnectedPlug(spec *seccomp.Spec func (iface *greengrassSupportInterface) UDevConnectedPlug(spec *udev.Specification, plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) error { var flavor string - _ = plug.Attr(flavorAttrStr, &flavor) + _ = plug.Attr("flavor", &flavor) switch flavor { - case "", flavorContainer: + case "", "legacy-container": // default containerization controls the device cgroup spec.SetControlsDeviceCgroup() - case falvorProcess: - // process mode does not control the device cgroup + case "no-container": + // no-container does not control the device cgroup } return nil diff --git a/interfaces/builtin/greengrass_support_test.go b/interfaces/builtin/greengrass_support_test.go index 215b3b44dee..4f0057ab2e6 100644 --- a/interfaces/builtin/greengrass_support_test.go +++ b/interfaces/builtin/greengrass_support_test.go @@ -62,25 +62,25 @@ slots: const ggMockPlugSnapInfoYaml = `name: other version: 1.0 plugs: - greengrass-support-container-mode: + greengrass-support-legacy-container: interface: greengrass-support - flavor: container + flavor: legacy-container apps: app2: command: foo - plugs: [greengrass-support-container-mode, greengrass-support, network-control] + plugs: [greengrass-support-legacy-container, greengrass-support, network-control] ` const ggProcessModeMockPlugSnapInfoYaml = `name: other version: 1.0 plugs: - greengrass-support-process-mode: + greengrass-support-no-container: interface: greengrass-support - flavor: process + flavor: no-container apps: app2: command: foo - plugs: [greengrass-support-process-mode, network-control] + plugs: [greengrass-support-no-container, network-control] ` var _ = Suite(&GreengrassSupportInterfaceSuite{ @@ -93,9 +93,9 @@ 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-process-mode") + s.processModePlug, s.processModePlugInfo = MockConnectedPlug(c, ggProcessModeMockPlugSnapInfoYaml, nil, "greengrass-support-no-container") - s.containerModePlug, s.containerModePlugInfo = MockConnectedPlug(c, ggMockPlugSnapInfoYaml, nil, "greengrass-support-container-mode") + s.containerModePlug, s.containerModePlugInfo = MockConnectedPlug(c, ggMockPlugSnapInfoYaml, nil, "greengrass-support-legacy-container") } From 2a007a89964ba34660fdd1225262653e980cfd07 Mon Sep 17 00:00:00 2001 From: James Henstridge Date: Mon, 4 Jan 2021 15:29:36 +0800 Subject: [PATCH 6/9] run-checks: update to match new argument syntax of ineffassign --- run-checks | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-checks b/run-checks index 5a6e67243a4..4e265ed8d1a 100755 --- a/run-checks +++ b/run-checks @@ -246,7 +246,7 @@ if [ "$STATIC" = 1 ]; then go get -u github.com/gordonklaus/ineffassign fi # ineffassign knows about ignoring vendor/ \o/ - ineffassign . + ineffassign ./... echo "Checking for naked returns" if ! command -v nakedret >/dev/null; then From 4d3de63f3b051ac190ec8eb0c4f2e3f6cf4f45d3 Mon Sep 17 00:00:00 2001 From: James Henstridge Date: Mon, 4 Jan 2021 15:58:36 +0800 Subject: [PATCH 7/9] run-checks: don't run ineffassign static check with Go < 1.12 --- run-checks | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/run-checks b/run-checks index 4e265ed8d1a..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 From e544986280d52fb7345b8ad0e845ed04b0da2182 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Mon, 4 Jan 2021 11:56:28 +0100 Subject: [PATCH 8/9] tests/main/cohorts: replace yq with a Python snippet The yq tool changed its command line arguments and the test broke. Try not to depend on external tools and use a simple Python snippet to extract the cohort key. Signed-off-by: Maciej Borzecki --- tests/main/cohorts/task.yaml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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:" From 0b206e1825e101ba269953942080c6163f39cef1 Mon Sep 17 00:00:00 2001 From: Maciej Borzecki Date: Mon, 4 Jan 2021 12:16:18 +0100 Subject: [PATCH 9/9] tests/lib/snaps/test-snapd-sh: add cmd command So that we don't have to deal with silly quoting when calling test-snapd-sh.sh. Signed-off-by: Maciej Borzecki --- tests/lib/snaps/test-snapd-sh/bin/cmd | 6 ++++++ tests/lib/snaps/test-snapd-sh/meta/snap.yaml | 2 ++ 2 files changed, 8 insertions(+) create mode 100755 tests/lib/snaps/test-snapd-sh/bin/cmd 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