Resolve users and groups through NSS in a shared getent package

client/internal/elevate/trusted_unix.go

@@ -3,23 +3,18 @@
 package elevate
 
 import (
-	"bufio"
 	"errors"
 	"fmt"
 	"os"
-	"os/user"
 	"path/filepath"
 	"slices"
 	"strconv"
-	"strings"
 	"syscall"
 
 	log "github.com/sirupsen/logrus"
-)
 
-// groupFile lists which accounts are in which group, for the membership a user
-// private group's name does not state: see groupHasOtherMembers.
-const groupFile = "/etc/group"
+	"github.com/netbirdio/netbird/client/internal/getent"
+)
 
 // checkOnlyOwnerWritable reports an error unless path, and every directory leading
 // to it, is owned by either root or this user and writable by nobody who could not
@@ -89,12 +84,12 @@ func groupWriteAllowed(uid, gid uint32) bool {
 		return true
 	}
 
-	group, err := user.LookupGroupId(strconv.FormatUint(uint64(gid), 10))
+	group, err := getent.LookupGroupID(strconv.FormatUint(uint64(gid), 10))
 	if err != nil {
 		log.Debugf("cannot look up group %d, treating it as shared: %v", gid, err)
 		return false
 	}
-	owner, err := user.LookupId(strconv.FormatUint(uint64(uid), 10))
+	owner, err := getent.LookupUserID(strconv.FormatUint(uint64(uid), 10))
 	if err != nil {
 		log.Debugf("cannot look up uid %d, treating its group as shared: %v", uid, err)
 		return false
@@ -103,7 +98,7 @@ func groupWriteAllowed(uid, gid uint32) bool {
 	if group.Name != owner.Username {
 		return false
 	}
-	return !groupHasOtherMembers(groupFile, group.Name, owner.Username)
+	return !groupHasOtherMembers(group.Name, owner.Username)
 }
 
 // groupHasOtherMembers reports whether the group lists a member besides owner.
@@ -111,36 +106,14 @@ func groupWriteAllowed(uid, gid uint32) bool {
 // Sharing the owner's name is what a user private group is recognised by, and it
 // says nothing about who is in it: a group that has since gained a member is
 // still named that way, and that member can write whatever the group can. So the
-// membership is read rather than assumed. A group this file does not describe,
-// because it comes from LDAP or another NSS source, cannot be answered here and
-// leaves the name as the only thing to go on.
-func groupHasOtherMembers(path, name, owner string) bool {
-	file, err := os.Open(path)
+// membership is read rather than assumed. A group whose members cannot be
+// listed, because no source on this host describes it, leaves the name as the
+// only thing to go on.
+func groupHasOtherMembers(name, owner string) bool {
+	members, err := getent.GroupMembers(name)
 	if err != nil {
-		log.Debugf("cannot read %s for the members of group %q: %v", path, name, err)
+		log.Debugf("cannot list the members of group %q, going by its name alone: %v", name, err)
 		return false
 	}
-	defer func() {
-		if err := file.Close(); err != nil {
-			log.Debugf("close %s: %v", path, err)
-		}
-	}()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		// name:password:gid:member,member
-		fields := strings.Split(scanner.Text(), ":")
-		if len(fields) < 4 || fields[0] != name {
-			continue
-		}
-		for member := range strings.SplitSeq(fields[3], ",") {
-			if member != "" && member != owner {
-				return true
-			}
-		}
-	}
-	if err := scanner.Err(); err != nil {
-		log.Debugf("read %s: %v", path, err)
-	}
-	return false
+	return slices.ContainsFunc(members, func(member string) bool { return member != owner })
 }

client/internal/elevate/trusted_unix_test.go

@@ -93,41 +93,13 @@ func TestCheckOnlyOwnerWritableAcceptsOwnPrivateGroup(t *testing.T) {
 	assert.NoError(t, err, "group write in the owner's own private group reaches nobody else")
 }
 
-// A group that shares its owner's name but has gained another member is no longer
-// private, and its write access reaches an account that could not elevate.
-func TestGroupHasOtherMembers(t *testing.T) {
-	tests := []struct {
-		name  string
-		entry string
-		want  bool
-	}{
-		{name: "no members", entry: "vma:x:1000:"},
-		{name: "only the owner", entry: "vma:x:1000:vma"},
-		{name: "another member", entry: "vma:x:1000:bob", want: true},
-		{name: "the owner and another", entry: "vma:x:1000:vma,bob", want: true},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			path := filepath.Join(t.TempDir(), "group")
-			body := "root:x:0:\n" + tt.entry + "\nsudo:x:27:vma\n"
-			require.NoError(t, os.WriteFile(path, []byte(body), 0o644), "write the group file")
-
-			assert.Equal(t, tt.want, groupHasOtherMembers(path, "vma", "vma"), "entry %q", tt.entry)
-		})
-	}
-}
-
-// A group file that says nothing about the group leaves the name as the only thing
-// to go on, so the private-group allowance stands rather than collapsing on every
-// host whose groups come from LDAP.
+// A group whose membership no source can answer for leaves the name as the only
+// thing to go on, so the private-group allowance stands rather than collapsing on
+// every host whose groups come from an unreadable source. The membership listing
+// itself lives in the getent package and is tested there.
 func TestGroupHasOtherMembersTolerantOfAnUnknownGroup(t *testing.T) {
-	path := filepath.Join(t.TempDir(), "group")
-	require.NoError(t, os.WriteFile(path, []byte("root:x:0:\n"), 0o644), "write the group file")
-
-	assert.False(t, groupHasOtherMembers(path, "vma", "vma"), "a group the file does not describe")
-	assert.False(t, groupHasOtherMembers(filepath.Join(t.TempDir(), "absent"), "vma", "vma"),
-		"no group file at all")
+	assert.False(t, groupHasOtherMembers("nonexistent_group_xyzzy_12345", "vma"),
+		"a group no source describes")
 }
 
 // A writable directory is as good as a writable file: whoever can write the
@@ -171,7 +143,7 @@ func requirePrivatePrimaryGroup(t *testing.T) {
 	if group.Name != self.Username {
 		t.Skipf("the test user's primary group is %q, not their own, so there is nothing to assert here", group.Name)
 	}
-	if groupHasOtherMembers(groupFile, group.Name, self.Username) {
+	if groupHasOtherMembers(group.Name, self.Username) {
 		t.Skipf("group %q has other members, so it is not a private group", group.Name)
 	}
 }