cmd/{snap-seccomp,snap-confine-ns},osutil,interfaces/account_control: workaround unit test failures on Arch

cmd/snap-seccomp/main_test.go

@@ -350,8 +350,9 @@ func (s *snapSeccompSuite) TestUnrestricted(c *C) {
 //    {"read >=2", "read;native;1", main.SeccompRetKill},
 //    {"read >=2", "read;native;0", main.SeccompRetKill},
 func (s *snapSeccompSuite) TestCompile(c *C) {
-	// The 'shadow' group is different in different distributions
-	shadowGid, err := osutil.FindGid("shadow")
+	// The 'shadow' group is different in different distributions, instead
+	// of second guessing group name, look at the owner of /etc/shadow
+	group, err := osutil.FindGroupOwning("/etc/shadow")
 	c.Assert(err, IsNil)
 
 	for _, t := range []struct {
@@ -447,11 +448,16 @@ func (s *snapSeccompSuite) TestCompile(c *C) {
 		{"ioctl - TIOCSTI", "ioctl;native;-,TIOCSTI", main.SeccompRetAllow},
 		{"ioctl - TIOCSTI", "ioctl;native;-,99", main.SeccompRetKill},
 
-		// u:root g:shadow
-		{"fchown - u:root g:shadow", fmt.Sprintf("fchown;native;-,0,%d", shadowGid), main.SeccompRetAllow},
-		{"fchown - u:root g:shadow", fmt.Sprintf("fchown;native;-,99,%d", shadowGid), main.SeccompRetKill},
-		{"chown - u:root g:shadow", fmt.Sprintf("chown;native;-,0,%d", shadowGid), main.SeccompRetAllow},
-		{"chown - u:root g:shadow", fmt.Sprintf("chown;native;-,99,%d", shadowGid), main.SeccompRetKill},
+		// u:root g:shadow - group used here depends on the actual owner
+		// of /etc/shadow
+		{fmt.Sprintf("fchown - u:root g:%s", group.Name),
+			fmt.Sprintf("fchown;native;-,0,%v", group.Gid), main.SeccompRetAllow},
+		{fmt.Sprintf("fchown - u:root g:%s", group.Name),
+			fmt.Sprintf("fchown;native;-,99,%v", group.Gid), main.SeccompRetKill},
+		{fmt.Sprintf("chown - u:root g:%s", group.Name),
+			fmt.Sprintf("chown;native;-,0,%v", group.Gid), main.SeccompRetAllow},
+		{fmt.Sprintf("chown - u:root g:%s", group.Name),
+			fmt.Sprintf("chown;native;-,99,%v", group.Gid), main.SeccompRetKill},
 	} {
 		s.runBpf(c, t.seccompWhitelist, t.bpfInput, t.expected)
 	}
@@ -569,7 +575,7 @@ func (s *snapSeccompSuite) TestCompileBadInput(c *C) {
 		{"setgid g:b@d|npu+", `cannot parse line: cannot parse token "g:b@d|npu+" \(line "setgid g:b@d|npu+"\): "b@d|npu+" must be a valid group name`},
 		{"setgid g:snap.bad", `cannot parse line: cannot parse token "g:snap.bad" \(line "setgid g:snap.bad"\): "snap.bad" must be a valid group name`},
 		{"setgid G:root", `cannot parse line: cannot parse token "G:root" .*`},
-		{"setgid g:nonexistent", `cannot parse line: cannot parse token "g:nonexistent" \(line "setgid g:nonexistent"\): group: unknown group nonexistent`},
+		{"setgid g:nonexistent", `cannot parse line: cannot parse token "g:nonexistent" \(line "setgid g:nonexistent"\): group: cannot find group nonexistent`},
 	} {
 		outPath := filepath.Join(c.MkDir(), "bpf")
 		err := main.Compile([]byte(t.inp), outPath)
@@ -741,17 +747,24 @@ func (s *snapSeccompSuite) TestRestrictionsWorkingArgsTermios(c *C) {
 }
 
 func (s *snapSeccompSuite) TestRestrictionsWorkingArgsUidGid(c *C) {
+	// while 'root' user usually has uid 0, 'daemon' user uid may vary
+	// across distributions, best lookup the uid directly
+	daemonUid, err := osutil.FindUid("daemon")
+	c.Assert(err, IsNil)
+
 	for _, t := range []struct {
 		seccompWhitelist string
 		bpfInput         string
 		expected         int
 	}{
-		// good input. 'root' and 'daemon' are guaranteed to be '0' and
-		// '1' respectively
+		// good input. 'root' is guaranteed to be '0' and 'daemon' uid
+		// was determined at runtime
 		{"setuid u:root", "setuid;native;0", main.SeccompRetAllow},
-		{"setuid u:daemon", "setuid;native;1", main.SeccompRetAllow},
+		{"setuid u:daemon", fmt.Sprintf("setuid;native;%v", daemonUid),
+			main.SeccompRetAllow},
 		{"setgid g:root", "setgid;native;0", main.SeccompRetAllow},
-		{"setgid g:daemon", "setgid;native;1", main.SeccompRetAllow},
+		{"setgid g:daemon", fmt.Sprintf("setgid;native;%v", daemonUid),
+			main.SeccompRetAllow},
 		// bad input
 		{"setuid u:root", "setuid;native;99", main.SeccompRetKill},
 		{"setuid u:daemon", "setuid;native;99", main.SeccompRetKill},

cmd/snap-update-ns/entry_test.go

@@ -20,6 +20,7 @@
 package main_test
 
 import (
+	"fmt"
 	"math"
 	"os"
 
@@ -28,6 +29,7 @@ import (
 	update "github.com/snapcore/snapd/cmd/snap-update-ns"
 	"github.com/snapcore/snapd/interfaces/mount"
 	"github.com/snapcore/snapd/osutil"
+	"github.com/snapcore/snapd/testutil"
 )
 
 type entrySuite struct{}
@@ -105,10 +107,24 @@ func (s *entrySuite) TestXSnapdGID(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(gid, Equals, uint64(0))
 
-	// Group is parsed from the x-snapd.gid = option.
-	nogroupGID, err := osutil.FindGid("nogroup")
-	c.Assert(err, IsNil)
-	e = &mount.Entry{Options: []string{"x-snapd.gid=nogroup"}}
+	// Group is parsed from the x-snapd-group= option.
+	var nogroup string
+	var nogroupGID uint64
+	// try to cover differences between distributions and find a suitable
+	// 'nogroup' like group, eg. Ubuntu uses 'nogroup' while Arch uses
+	// 'nobody'
+	for _, grp := range []string{"nogroup", "nobody"} {
+		if gid, err := osutil.FindGid(grp); err == nil {
+			nogroup = grp
+			nogroupGID = gid
+			break
+		}
+	}
+	c.Assert([]string{"nogroup", "nobody"}, testutil.Contains, nogroup)
+
+	e = &mount.Entry{
+		Options: []string{fmt.Sprintf("x-snapd.gid=%s", nogroup)},
+	}
 	gid, err = update.XSnapdGID(e)
 	c.Assert(err, IsNil)
 	c.Assert(gid, Equals, nogroupGID)

dirs/dirs_test.go

@@ -20,6 +20,7 @@
 package dirs_test
 
 import (
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
@@ -78,6 +79,9 @@ func (s *DirsTestSuite) TestClassicConfinementSymlinkWorkaround(c *C) {
 }
 
 func (s *DirsTestSuite) TestClassicConfinementSupportOnSpecificDistributions(c *C) {
+	// the test changes RootDir, restore correct one when retuning
+	defer dirs.SetRootDir("/")
+
 	for _, t := range []struct {
 		ID       string
 		IDLike   []string
@@ -93,7 +97,15 @@ func (s *DirsTestSuite) TestClassicConfinementSupportOnSpecificDistributions(c *
 	} {
 		reset := release.MockReleaseInfo(&release.OS{ID: t.ID, IDLike: t.IDLike})
 		defer reset()
-		dirs.SetRootDir("/")
+
+		// make a new root directory each time to isolate the test from
+		// local filesystem state and any previous test runs
+		altRoot, err := ioutil.TempDir("",
+			"TestClassicConfinementSupportOnSpecificDistributions")
+		c.Assert(err, IsNil)
+		defer os.RemoveAll(altRoot)
+
+		dirs.SetRootDir(altRoot)
 		c.Check(dirs.SupportsClassicConfinement(), Equals, t.Expected, Commentf("unexpected result for %v", t.ID))
 	}
 }

interfaces/builtin/account_control.go

@@ -19,6 +19,14 @@
 
 package builtin
 
+import (
+	"strings"
+
+	"github.com/snapcore/snapd/interfaces"
+	"github.com/snapcore/snapd/interfaces/seccomp"
+	"github.com/snapcore/snapd/osutil"
+)
+
 const accountControlSummary = `allows managing non-system user accounts`
 
 const accountControlBaseDeclarationSlots = `
@@ -56,26 +64,55 @@ capability fsetid,
 /var/log/faillog rwk,
 `
 
-// Needed because useradd uses a netlink socket
-const accountControlConnectedPlugSecComp = `
-# useradd requires chowning to 'shadow'
-fchown - u:root g:shadow
-fchown32 - u:root g:shadow
+// Needed because useradd uses a netlink socket, {{group}} is used as a
+// placeholder argument for the actual name of a group owning /etc/shadow
+const accountControlConnectedPlugSecCompTemplate = `
+# useradd requires chowning to '{{group}}'
+fchown - u:root g:{{group}}
+fchown32 - u:root g:{{group}}
 
 # from libaudit1
 bind
 socket AF_NETLINK - NETLINK_AUDIT
 `
 
+type accountControlInterface struct {
+	commonInterface
+}
+
+func makeAccountControlSecCompSnippet() (string, error) {
+	group, err := osutil.FindGroupOwning("/etc/shadow")
+	if err != nil {
+		return "", err
+	}
+
+	snippet := strings.Replace(accountControlConnectedPlugSecCompTemplate,
+		"{{group}}", group.Name, -1)
+
+	return snippet, nil
+}
+
+func (iface *accountControlInterface) SecCompConnectedPlug(spec *seccomp.Specification, plug *interfaces.Plug, Attrs map[string]interface{}, slot *interfaces.Slot, slotAttrs map[string]interface{}) error {
+	snippet, err := makeAccountControlSecCompSnippet()
+	if err != nil {
+		return err
+	}
+	spec.AddSnippet(snippet)
+	return nil
+}
+
 func init() {
-	registerIface(&commonInterface{
-		name:                  "account-control",
-		summary:               accountControlSummary,
-		implicitOnCore:        true,
-		implicitOnClassic:     true,
-		baseDeclarationSlots:  accountControlBaseDeclarationSlots,
-		connectedPlugAppArmor: accountControlConnectedPlugAppArmor,
-		connectedPlugSecComp:  accountControlConnectedPlugSecComp,
-		reservedForOS:         true,
+	registerIface(&accountControlInterface{
+		commonInterface: commonInterface{
+			name:                  "account-control",
+			summary:               accountControlSummary,
+			implicitOnCore:        true,
+			implicitOnClassic:     true,
+			baseDeclarationSlots:  accountControlBaseDeclarationSlots,
+			connectedPlugAppArmor: accountControlConnectedPlugAppArmor,
+			// handled by SecCompConnectedPlug
+			connectedPlugSecComp: "",
+			reservedForOS:        true,
+		},
 	})
 }

interfaces/builtin/account_control_test.go

@@ -20,12 +20,15 @@
 package builtin_test
 
 import (
+	"fmt"
+
 	. "gopkg.in/check.v1"
 
 	"github.com/snapcore/snapd/interfaces"
 	"github.com/snapcore/snapd/interfaces/apparmor"
 	"github.com/snapcore/snapd/interfaces/builtin"
 	"github.com/snapcore/snapd/interfaces/seccomp"
+	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/snap/snaptest"
 	"github.com/snapcore/snapd/testutil"
@@ -99,7 +102,11 @@ func (s *AccountControlSuite) TestUsedSecuritySystems(c *C) {
 	err = seccompSpec.AddConnectedPlug(s.iface, s.plug, nil, s.slot, nil)
 	c.Assert(err, IsNil)
 	c.Assert(seccompSpec.SecurityTags(), DeepEquals, []string{"snap.other.app2"})
-	c.Check(seccompSpec.SnippetForTag("snap.other.app2"), testutil.Contains, "\nfchown - u:root g:shadow\n")
+	group, err := osutil.FindGroupOwning("/etc/shadow")
+	c.Assert(err, IsNil)
+	c.Assert([]string{"shadow", "root"}, testutil.Contains, group.Name)
+	c.Check(seccompSpec.SnippetForTag("snap.other.app2"), testutil.Contains,
+		fmt.Sprintf("\nfchown - u:root g:%s\n", group.Name))
 }
 
 func (s *AccountControlSuite) TestInterfaces(c *C) {