From 2f7bcb9bf0fda24f1cd74a78caa8138258711312 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Thu, 20 Aug 2026 15:27:40 +0200 Subject: [PATCH] Resolve users and groups through NSS in a shared getent package --- client/internal/elevate/trusted_unix.go | 51 +--- client/internal/elevate/trusted_unix_test.go | 42 +-- client/internal/getent/cgo_unix.go | 36 +++ client/internal/getent/getent.go | 6 + .../server => internal/getent}/getent_test.go | 105 ++++---- client/internal/getent/nocgo_unix.go | 110 ++++++++ client/internal/getent/unix.go | 224 ++++++++++++++++ .../getent/unix_test.go} | 247 +++++++++++------- client/internal/getent/windows.go | 36 +++ client/ssh/server/getent_cgo_unix.go | 24 -- client/ssh/server/getent_nocgo_unix.go | 74 ------ client/ssh/server/getent_unix.go | 127 --------- client/ssh/server/getent_windows.go | 26 -- client/ssh/server/shell.go | 8 +- client/ssh/server/shell_unix_test.go | 94 +++++++ client/ssh/server/user_utils.go | 6 +- client/ssh/server/userswitching_unix.go | 4 +- 17 files changed, 735 insertions(+), 485 deletions(-) create mode 100644 client/internal/getent/cgo_unix.go create mode 100644 client/internal/getent/getent.go rename client/{ssh/server => internal/getent}/getent_test.go (53%) create mode 100644 client/internal/getent/nocgo_unix.go create mode 100644 client/internal/getent/unix.go rename client/{ssh/server/getent_unix_test.go => internal/getent/unix_test.go} (63%) create mode 100644 client/internal/getent/windows.go delete mode 100644 client/ssh/server/getent_cgo_unix.go delete mode 100644 client/ssh/server/getent_nocgo_unix.go delete mode 100644 client/ssh/server/getent_unix.go delete mode 100644 client/ssh/server/getent_windows.go create mode 100644 client/ssh/server/shell_unix_test.go diff --git a/client/internal/elevate/trusted_unix.go b/client/internal/elevate/trusted_unix.go index d7a691164..c47bffe55 100644 --- a/client/internal/elevate/trusted_unix.go +++ b/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 }) } diff --git a/client/internal/elevate/trusted_unix_test.go b/client/internal/elevate/trusted_unix_test.go index e69d5711e..a13464dd8 100644 --- a/client/internal/elevate/trusted_unix_test.go +++ b/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) } } diff --git a/client/internal/getent/cgo_unix.go b/client/internal/getent/cgo_unix.go new file mode 100644 index 000000000..2853aafff --- /dev/null +++ b/client/internal/getent/cgo_unix.go @@ -0,0 +1,36 @@ +//go:build cgo && !osusergo && !windows + +package getent + +import "os/user" + +// Built with cgo, os/user resolves through libc (getpwnam_r and friends), +// which goes through the host's NSS stack natively. Whatever it fails to +// find, the getent command would not find either, so there is nothing to +// fall back to. + +// LookupUser looks up a user by name. +func LookupUser(username string) (*user.User, error) { + return user.Lookup(username) +} + +// LookupUserID looks up a user by UID. +func LookupUserID(uid string) (*user.User, error) { + return user.LookupId(uid) +} + +// CurrentUser returns the user this process runs as. +func CurrentUser() (*user.User, error) { + return user.Current() +} + +// LookupGroupID looks up a group by GID. +func LookupGroupID(gid string) (*user.Group, error) { + return user.LookupGroupId(gid) +} + +// GroupIDs returns the IDs of the groups the user is a member of; libc's +// getgrouplist handles NSS groups natively. +func GroupIDs(u *user.User) ([]string, error) { + return u.GroupIds() +} diff --git a/client/internal/getent/getent.go b/client/internal/getent/getent.go new file mode 100644 index 000000000..9cfebe64b --- /dev/null +++ b/client/internal/getent/getent.go @@ -0,0 +1,6 @@ +// Package getent resolves users and groups through the host's NSS stack. +// Built without cgo, os/user reads /etc/passwd and /etc/group alone and misses +// anything LDAP, SSSD or winbind provide; the getent and id commands resolve +// through NSS whatever the build. The lookups here try the standard library +// first, which needs no subprocess, and fall back to those commands. +package getent diff --git a/client/ssh/server/getent_test.go b/client/internal/getent/getent_test.go similarity index 53% rename from client/ssh/server/getent_test.go rename to client/internal/getent/getent_test.go index 5eac2fdbe..8176eba36 100644 --- a/client/ssh/server/getent_test.go +++ b/client/internal/getent/getent_test.go @@ -1,4 +1,4 @@ -package server +package getent import ( "os/user" @@ -10,38 +10,48 @@ import ( "github.com/stretchr/testify/require" ) -func TestLookupWithGetent_CurrentUser(t *testing.T) { +func TestLookupUser_CurrentUser(t *testing.T) { // The current user should always be resolvable on any platform current, err := user.Current() require.NoError(t, err) - u, err := lookupWithGetent(current.Username) + u, err := LookupUser(current.Username) require.NoError(t, err) assert.Equal(t, current.Username, u.Username) assert.Equal(t, current.Uid, u.Uid) assert.Equal(t, current.Gid, u.Gid) } -func TestLookupWithGetent_NonexistentUser(t *testing.T) { - _, err := lookupWithGetent("nonexistent_user_xyzzy_12345") +func TestLookupUser_NonexistentUser(t *testing.T) { + _, err := LookupUser("nonexistent_user_xyzzy_12345") require.Error(t, err, "should fail for nonexistent user") } -func TestCurrentUserWithGetent(t *testing.T) { +func TestLookupUserID_CurrentUser(t *testing.T) { + current, err := user.Current() + require.NoError(t, err) + + u, err := LookupUserID(current.Uid) + require.NoError(t, err) + assert.Equal(t, current.Username, u.Username) + assert.Equal(t, current.Uid, u.Uid) +} + +func TestCurrentUser(t *testing.T) { stdUser, err := user.Current() require.NoError(t, err) - u, err := currentUserWithGetent() + u, err := CurrentUser() require.NoError(t, err) assert.Equal(t, stdUser.Uid, u.Uid) assert.Equal(t, stdUser.Username, u.Username) } -func TestGroupIdsWithFallback_CurrentUser(t *testing.T) { +func TestGroupIDs_CurrentUser(t *testing.T) { current, err := user.Current() require.NoError(t, err) - groups, err := groupIdsWithFallback(current) + groups, err := GroupIDs(current) require.NoError(t, err) require.NotEmpty(t, groups, "current user should have at least one group") @@ -53,32 +63,30 @@ func TestGroupIdsWithFallback_CurrentUser(t *testing.T) { } } -func TestGetShellFromGetent_CurrentUser(t *testing.T) { - if runtime.GOOS == "windows" { - // Windows stub always returns empty, which is correct - shell := getShellFromGetent("1000") - assert.Empty(t, shell, "Windows stub should return empty") - return - } - +func TestUserShell_CurrentUser(t *testing.T) { current, err := user.Current() require.NoError(t, err) - // getent may not be available on all systems (e.g., macOS without Homebrew getent) - shell := getShellFromGetent(current.Uid) + // getent may not be available on all systems (e.g., macOS without + // Homebrew getent), and Windows has no login shells at all. + shell, err := UserShell(current.Uid) + if err != nil { + t.Logf("UserShell failed, getent may not be available: %v", err) + return + } if shell == "" { - t.Log("getShellFromGetent returned empty, getent may not be available") + t.Log("UserShell returned empty, the user has no shell set") return } assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) } -func TestLookupWithGetent_RootUser(t *testing.T) { +func TestLookupUser_RootUser(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("no root user on Windows") } - u, err := lookupWithGetent("root") + u, err := LookupUser("root") if err != nil { t.Skip("root user not available on this system") } @@ -86,25 +94,25 @@ func TestLookupWithGetent_RootUser(t *testing.T) { } // TestIntegration_FullLookupChain exercises the complete user lookup chain -// against the real system, testing that all wrappers (lookupWithGetent, -// currentUserWithGetent, groupIdsWithFallback, getShellFromGetent) produce -// consistent and correct results when composed together. +// against the real system, testing that all wrappers (LookupUser, +// CurrentUser, GroupIDs, UserShell) produce consistent and correct results +// when composed together. func TestIntegration_FullLookupChain(t *testing.T) { - // Step 1: currentUserWithGetent must resolve the running user. - current, err := currentUserWithGetent() - require.NoError(t, err, "currentUserWithGetent must resolve the running user") + // Step 1: CurrentUser must resolve the running user. + current, err := CurrentUser() + require.NoError(t, err, "CurrentUser must resolve the running user") require.NotEmpty(t, current.Uid) require.NotEmpty(t, current.Username) - // Step 2: lookupWithGetent by the same username must return matching identity. - byName, err := lookupWithGetent(current.Username) + // Step 2: LookupUser by the same username must return matching identity. + byName, err := LookupUser(current.Username) require.NoError(t, err) assert.Equal(t, current.Uid, byName.Uid, "lookup by name should return same UID") assert.Equal(t, current.Gid, byName.Gid, "lookup by name should return same GID") assert.Equal(t, current.HomeDir, byName.HomeDir, "lookup by name should return same home") - // Step 3: groupIdsWithFallback must return at least the primary GID. - groups, err := groupIdsWithFallback(current) + // Step 3: GroupIDs must return at least the primary GID. + groups, err := GroupIDs(current) require.NoError(t, err) require.NotEmpty(t, groups, "user must have at least one group") @@ -119,29 +127,20 @@ func TestIntegration_FullLookupChain(t *testing.T) { } } assert.True(t, foundPrimary, "primary GID %s should appear in supplementary groups", current.Gid) - - // Step 4: getShellFromGetent should either return a valid shell path or empty - // (empty is OK when getent is not available, e.g. macOS without Homebrew getent). - if runtime.GOOS != "windows" { - shell := getShellFromGetent(current.Uid) - if shell != "" { - assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) - } - } } // TestIntegration_LookupAndGroupsConsistency verifies that a user resolved via -// lookupWithGetent can have their groups resolved via groupIdsWithFallback, -// testing the handoff between the two functions as used by the SSH server. +// LookupUser can have their groups resolved via GroupIDs, testing the handoff +// between the two functions as used by the SSH server. func TestIntegration_LookupAndGroupsConsistency(t *testing.T) { current, err := user.Current() require.NoError(t, err) // Simulate the SSH server flow: lookup user, then get their groups. - resolved, err := lookupWithGetent(current.Username) + resolved, err := LookupUser(current.Username) require.NoError(t, err) - groups, err := groupIdsWithFallback(resolved) + groups, err := GroupIDs(resolved) require.NoError(t, err) require.NotEmpty(t, groups, "resolved user must have groups") @@ -154,19 +153,3 @@ func TestIntegration_LookupAndGroupsConsistency(t *testing.T) { } } } - -// TestIntegration_ShellLookupChain tests the full shell resolution chain -// (getShellFromPasswd -> getShellFromGetent -> $SHELL -> default) on Unix. -func TestIntegration_ShellLookupChain(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Unix shell lookup not applicable on Windows") - } - - current, err := user.Current() - require.NoError(t, err) - - // getUserShell is the top-level function used by the SSH server. - shell := getUserShell(current.Uid) - require.NotEmpty(t, shell, "getUserShell must always return a shell") - assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) -} diff --git a/client/internal/getent/nocgo_unix.go b/client/internal/getent/nocgo_unix.go new file mode 100644 index 000000000..94d8ea6a9 --- /dev/null +++ b/client/internal/getent/nocgo_unix.go @@ -0,0 +1,110 @@ +//go:build (!cgo || osusergo) && !windows + +package getent + +import ( + "os" + "os/user" + "strconv" + + log "github.com/sirupsen/logrus" +) + +// Without cgo, os/user only reads /etc/passwd and /etc/group and misses +// NSS-provided users and groups; the getent and id commands go through the +// host's NSS stack. + +// LookupUser looks up a user by name, falling back to getent if os/user fails. +func LookupUser(username string) (*user.User, error) { + u, err := user.Lookup(username) + if err == nil { + return u, nil + } + + stdErr := err + log.Debugf("os/user.Lookup(%q) failed, trying getent: %v", username, err) + + u, _, getentErr := passwdLookup(username) + if getentErr != nil { + log.Debugf("getent fallback for %q also failed: %v", username, getentErr) + return nil, stdErr + } + return u, nil +} + +// LookupUserID looks up a user by UID, falling back to getent if os/user fails. +func LookupUserID(uid string) (*user.User, error) { + u, err := user.LookupId(uid) + if err == nil { + return u, nil + } + + stdErr := err + log.Debugf("os/user.LookupId(%q) failed, trying getent: %v", uid, err) + + u, _, getentErr := passwdLookup(uid) + if getentErr != nil { + log.Debugf("getent fallback for uid %s also failed: %v", uid, getentErr) + return nil, stdErr + } + return u, nil +} + +// CurrentUser returns the user this process runs as, falling back to getent +// if os/user fails. +func CurrentUser() (*user.User, error) { + u, err := user.Current() + if err == nil { + return u, nil + } + + stdErr := err + uid := strconv.Itoa(os.Getuid()) + log.Debugf("os/user.Current() failed, trying getent with UID %s: %v", uid, err) + + u, _, getentErr := passwdLookup(uid) + if getentErr != nil { + return nil, stdErr + } + return u, nil +} + +// LookupGroupID looks up a group by GID, falling back to getent if os/user +// fails. +func LookupGroupID(gid string) (*user.Group, error) { + g, err := user.LookupGroupId(gid) + if err == nil { + return g, nil + } + + stdErr := err + log.Debugf("os/user.LookupGroupId(%q) failed, trying getent: %v", gid, err) + + g, _, getentErr := groupLookup(gid) + if getentErr != nil { + log.Debugf("getent fallback for gid %s also failed: %v", gid, getentErr) + return nil, stdErr + } + return g, nil +} + +// GroupIDs returns the IDs of the groups the user is a member of. +// NOTE: unlike the lookups above, which try the standard library first, this +// intentionally tries `id -G` first because without cgo, user.GroupIds only +// reads /etc/group and silently returns incomplete results for NSS users +// (no error, just missing groups). The id command goes through NSS and +// returns the full set. +func GroupIDs(u *user.User) ([]string, error) { + ids, err := idGroups(u.Username) + if err == nil { + return ids, nil + } + + log.Debugf("id -G %q failed, falling back to user.GroupIds(): %v", u.Username, err) + + ids, stdErr := u.GroupIds() + if stdErr != nil { + return nil, stdErr + } + return ids, nil +} diff --git a/client/internal/getent/unix.go b/client/internal/getent/unix.go new file mode 100644 index 000000000..7d29810f5 --- /dev/null +++ b/client/internal/getent/unix.go @@ -0,0 +1,224 @@ +//go:build !windows + +package getent + +import ( + "bufio" + "context" + "fmt" + "os" + "os/exec" + "os/user" + "runtime" + "strings" + "time" + + log "github.com/sirupsen/logrus" +) + +const commandTimeout = 5 * time.Second + +// groupFile lists which accounts are in which group, for hosts where the +// getent command is not available (macOS ships without it). +const groupFile = "/etc/group" + +// UserShell returns the login shell getent reports for the user with this UID. +// It reaches shells that /etc/passwd does not list, because getent resolves +// through the host's NSS stack. +func UserShell(uid string) (string, error) { + _, shell, err := passwdLookup(uid) + if err != nil { + return "", err + } + return shell, nil +} + +// GroupMembers returns the names of the group's members: from getent, which +// resolves through NSS, or from /etc/group where getent is not available. A +// group neither source describes is an error; an empty member list is not, +// since accounts with the group as their primary one are not listed in it. +func GroupMembers(name string) ([]string, error) { + _, members, err := groupLookup(name) + if err == nil { + return members, nil + } + log.Debugf("getent cannot list group %q, reading %s: %v", name, groupFile, err) + return groupMembersFromFile(groupFile, name) +} + +// passwdLookup executes `getent passwd `, where query is a username or +// UID, and returns the user and login shell. +func passwdLookup(query string) (*user.User, string, error) { + out, err := run("passwd", query) + if err != nil { + return nil, "", err + } + return parsePasswd(string(out)) +} + +// groupLookup executes `getent group `, where query is a group name or +// GID, and returns the group and its member names. +func groupLookup(query string) (*user.Group, []string, error) { + out, err := run("group", query) + if err != nil { + return nil, nil, err + } + return parseGroup(string(out)) +} + +// run executes `getent ` with a timeout. +func run(database, key string) ([]byte, error) { + if !validateInput(key) { + return nil, fmt.Errorf("invalid getent input: %q", key) + } + + ctx, cancel := context.WithTimeout(context.Background(), commandTimeout) + defer cancel() + + out, err := exec.CommandContext(ctx, "getent", database, key).Output() + if err != nil { + return nil, fmt.Errorf("getent %s %s: %w", database, key, err) + } + return out, nil +} + +// parsePasswd parses getent passwd output: "name:x:uid:gid:gecos:home:shell" +func parsePasswd(output string) (*user.User, string, error) { + fields := strings.SplitN(strings.TrimSpace(output), ":", 8) + if len(fields) < 6 { + return nil, "", fmt.Errorf("unexpected getent output (need 6+ fields): %q", output) + } + + if fields[0] == "" || fields[2] == "" || fields[3] == "" { + return nil, "", fmt.Errorf("missing required fields in getent output: %q", output) + } + + var shell string + if len(fields) >= 7 { + shell = fields[6] + } + + return &user.User{ + Username: fields[0], + Uid: fields[2], + Gid: fields[3], + Name: fields[4], + HomeDir: fields[5], + }, shell, nil +} + +// parseGroup parses getent group output: "name:x:gid:member,member" +func parseGroup(output string) (*user.Group, []string, error) { + fields := strings.SplitN(strings.TrimSpace(output), ":", 4) + if len(fields) < 3 { + return nil, nil, fmt.Errorf("unexpected getent output (need 3+ fields): %q", output) + } + + if fields[0] == "" || fields[2] == "" { + return nil, nil, fmt.Errorf("missing required fields in getent output: %q", output) + } + + var members []string + if len(fields) >= 4 { + members = splitMembers(fields[3]) + } + return &user.Group{Name: fields[0], Gid: fields[2]}, members, nil +} + +func splitMembers(list string) []string { + var members []string + for member := range strings.SplitSeq(list, ",") { + if member != "" { + members = append(members, member) + } + } + return members +} + +// groupMembersFromFile finds the group's member list in a file of /etc/group's +// format. A group the file does not describe, because it comes from LDAP or +// another NSS source, is an error rather than an empty list. +func groupMembersFromFile(path, name string) ([]string, error) { + file, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("open %s: %w", path, err) + } + 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 + } + return splitMembers(fields[3]), nil + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("read %s: %w", path, err) + } + return nil, fmt.Errorf("%s does not describe group %q", path, name) +} + +// validateInput checks that the input is safe to pass to getent or id. +// Allows POSIX usernames, numeric IDs, and common NSS extensions +// (@ for Kerberos, $ for Samba, + for NIS compat). A leading hyphen is +// rejected so the input can never be parsed as a command-line flag. +func validateInput(input string) bool { + maxLen := 32 + if runtime.GOOS == "linux" { + maxLen = 256 + } + + if len(input) == 0 || len(input) > maxLen { + return false + } + + if input[0] == '-' { + return false + } + + for _, r := range input { + if isAllowedChar(r) { + continue + } + return false + } + return true +} + +func isAllowedChar(r rune) bool { + if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' { + return true + } + switch r { + case '.', '_', '-', '@', '+', '$': + return true + } + return false +} + +// idGroups runs `id -G ` and returns the space-separated group IDs. +func idGroups(username string) ([]string, error) { + if !validateInput(username) { + return nil, fmt.Errorf("invalid username for id command: %q", username) + } + + ctx, cancel := context.WithTimeout(context.Background(), commandTimeout) + defer cancel() + + out, err := exec.CommandContext(ctx, "id", "-G", username).Output() + if err != nil { + return nil, fmt.Errorf("id -G %s: %w", username, err) + } + + trimmed := strings.TrimSpace(string(out)) + if trimmed == "" { + return nil, fmt.Errorf("id -G %s: empty output", username) + } + return strings.Fields(trimmed), nil +} diff --git a/client/ssh/server/getent_unix_test.go b/client/internal/getent/unix_test.go similarity index 63% rename from client/ssh/server/getent_unix_test.go rename to client/internal/getent/unix_test.go index a73214e17..d7215c4a4 100644 --- a/client/ssh/server/getent_unix_test.go +++ b/client/internal/getent/unix_test.go @@ -1,10 +1,12 @@ //go:build !windows -package server +package getent import ( + "os" "os/exec" "os/user" + "path/filepath" "runtime" "strconv" "testing" @@ -13,7 +15,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestParseGetentPasswd(t *testing.T) { +func TestParsePasswd(t *testing.T) { tests := []struct { name string input string @@ -128,7 +130,7 @@ func TestParseGetentPasswd(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - u, shell, err := parseGetentPasswd(tt.input) + u, shell, err := parsePasswd(tt.input) if tt.wantErr { require.Error(t, err) if tt.errContains != "" { @@ -147,7 +149,119 @@ func TestParseGetentPasswd(t *testing.T) { } } -func TestValidateGetentInput(t *testing.T) { +func TestParseGroup(t *testing.T) { + tests := []struct { + name string + input string + wantGroup *user.Group + wantMembers []string + wantErr bool + }{ + { + name: "no members", + input: "vma:x:1000:\n", + wantGroup: &user.Group{Name: "vma", Gid: "1000"}, + }, + { + name: "one member", + input: "sudo:x:27:alice", + wantGroup: &user.Group{Name: "sudo", Gid: "27"}, + wantMembers: []string{"alice"}, + }, + { + name: "several members", + input: "docker:x:998:alice,bob\n", + wantGroup: &user.Group{Name: "docker", Gid: "998"}, + wantMembers: []string{"alice", "bob"}, + }, + { + name: "too few fields", + input: "bad:x", + wantErr: true, + }, + { + name: "empty group name", + input: ":x:1000:alice", + wantErr: true, + }, + { + name: "empty GID", + input: "vma:x::alice", + wantErr: true, + }, + { + name: "empty input", + input: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g, members, err := parseGroup(tt.input) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantGroup.Name, g.Name, "group name") + assert.Equal(t, tt.wantGroup.Gid, g.Gid, "GID") + assert.Equal(t, tt.wantMembers, members, "members") + }) + } +} + +func TestGroupMembersFromFile(t *testing.T) { + tests := []struct { + name string + entry string + want []string + }{ + {name: "no members", entry: "vma:x:1000:"}, + {name: "only the owner", entry: "vma:x:1000:vma", want: []string{"vma"}}, + {name: "two members", entry: "vma:x:1000:vma,bob", want: []string{"vma", "bob"}}, + } + + 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") + + members, err := groupMembersFromFile(path, "vma") + require.NoError(t, err, "entry %q", tt.entry) + assert.Equal(t, tt.want, members, "entry %q", tt.entry) + }) + } +} + +// A group the file does not describe, because it comes from LDAP or another +// NSS source, is an error rather than an empty member list: the caller must +// be able to tell "no members" from "no answer". +func TestGroupMembersFromFileUnknownGroup(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") + + _, err := groupMembersFromFile(path, "vma") + assert.Error(t, err, "a group the file does not describe") + + _, err = groupMembersFromFile(filepath.Join(t.TempDir(), "absent"), "vma") + assert.Error(t, err, "no group file at all") +} + +// GroupMembers on the root group, which every Unix has, whichever source +// answers for it. +func TestGroupMembers_RootGroup(t *testing.T) { + rootGroup := "root" + if runtime.GOOS == "darwin" { + rootGroup = "wheel" + } + + _, err := GroupMembers(rootGroup) + assert.NoError(t, err, "the %s group must be describable", rootGroup) +} + +func TestValidateInput(t *testing.T) { tests := []struct { name string input string @@ -180,7 +294,7 @@ func TestValidateGetentInput(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, validateGetentInput(tt.input)) + assert.Equal(t, tt.want, validateInput(tt.input)) }) } } @@ -193,12 +307,12 @@ func makeLongString(n int) string { return string(b) } -func TestRunGetent_RootUser(t *testing.T) { +func TestPasswdLookup_RootUser(t *testing.T) { if _, err := exec.LookPath("getent"); err != nil { t.Skip("getent not available on this system") } - u, shell, err := runGetent("root") + u, shell, err := passwdLookup("root") require.NoError(t, err) assert.Equal(t, "root", u.Username) assert.Equal(t, "0", u.Uid) @@ -206,44 +320,55 @@ func TestRunGetent_RootUser(t *testing.T) { assert.NotEmpty(t, shell, "root should have a shell") } -func TestRunGetent_ByUID(t *testing.T) { +func TestPasswdLookup_ByUID(t *testing.T) { if _, err := exec.LookPath("getent"); err != nil { t.Skip("getent not available on this system") } - u, _, err := runGetent("0") + u, _, err := passwdLookup("0") require.NoError(t, err) assert.Equal(t, "root", u.Username) assert.Equal(t, "0", u.Uid) } -func TestRunGetent_NonexistentUser(t *testing.T) { +func TestPasswdLookup_NonexistentUser(t *testing.T) { if _, err := exec.LookPath("getent"); err != nil { t.Skip("getent not available on this system") } - _, _, err := runGetent("nonexistent_user_xyzzy_12345") + _, _, err := passwdLookup("nonexistent_user_xyzzy_12345") assert.Error(t, err) } -func TestRunGetent_InvalidInput(t *testing.T) { - _, _, err := runGetent("") +func TestPasswdLookup_InvalidInput(t *testing.T) { + _, _, err := passwdLookup("") assert.Error(t, err) - _, _, err = runGetent("user\x00name") + _, _, err = passwdLookup("user\x00name") assert.Error(t, err) } -func TestRunGetent_NotAvailable(t *testing.T) { +func TestPasswdLookup_NotAvailable(t *testing.T) { if _, err := exec.LookPath("getent"); err == nil { t.Skip("getent is available, can't test missing case") } - _, _, err := runGetent("root") + _, _, err := passwdLookup("root") assert.Error(t, err, "should fail when getent is not installed") } -func TestRunIdGroups_CurrentUser(t *testing.T) { +func TestGroupLookup_RootGroup(t *testing.T) { + if _, err := exec.LookPath("getent"); err != nil { + t.Skip("getent not available on this system") + } + + g, _, err := groupLookup("0") + require.NoError(t, err) + assert.Equal(t, "0", g.Gid, "GID 0 resolves to the root group") + assert.NotEmpty(t, g.Name, "the root group has a name") +} + +func TestIdGroups_CurrentUser(t *testing.T) { if _, err := exec.LookPath("id"); err != nil { t.Skip("id not available on this system") } @@ -251,7 +376,7 @@ func TestRunIdGroups_CurrentUser(t *testing.T) { current, err := user.Current() require.NoError(t, err) - groups, err := runIdGroups(current.Username) + groups, err := idGroups(current.Username) require.NoError(t, err) require.NotEmpty(t, groups, "current user should have at least one group") @@ -261,20 +386,20 @@ func TestRunIdGroups_CurrentUser(t *testing.T) { } } -func TestRunIdGroups_NonexistentUser(t *testing.T) { +func TestIdGroups_NonexistentUser(t *testing.T) { if _, err := exec.LookPath("id"); err != nil { t.Skip("id not available on this system") } - _, err := runIdGroups("nonexistent_user_xyzzy_12345") + _, err := idGroups("nonexistent_user_xyzzy_12345") assert.Error(t, err) } -func TestRunIdGroups_InvalidInput(t *testing.T) { - _, err := runIdGroups("") +func TestIdGroups_InvalidInput(t *testing.T) { + _, err := idGroups("") assert.Error(t, err) - _, err = runIdGroups("user\x00name") + _, err = idGroups("user\x00name") assert.Error(t, err) } @@ -286,7 +411,7 @@ func TestGetentResultsMatchStdlib(t *testing.T) { current, err := user.Current() require.NoError(t, err) - getentUser, _, err := runGetent(current.Username) + getentUser, _, err := passwdLookup(current.Username) require.NoError(t, err) assert.Equal(t, current.Username, getentUser.Username, "username should match") @@ -303,7 +428,7 @@ func TestGetentResultsMatchStdlib_ByUID(t *testing.T) { current, err := user.Current() require.NoError(t, err) - getentUser, _, err := runGetent(current.Uid) + getentUser, _, err := passwdLookup(current.Uid) require.NoError(t, err) assert.Equal(t, current.Username, getentUser.Username, "username should match when looked up by UID") @@ -323,12 +448,12 @@ func TestIdGroupsMatchStdlib(t *testing.T) { t.Skip("os/user.GroupIds() not working, likely CGO_ENABLED=0") } - idGroups, err := runIdGroups(current.Username) + idGroupIDs, err := idGroups(current.Username) require.NoError(t, err) // Deduplicate both lists: id -G can return duplicates (e.g., root in Docker) // and ElementsMatch treats duplicates as distinct. - assert.ElementsMatch(t, uniqueStrings(stdGroups), uniqueStrings(idGroups), "id -G should return same groups as os/user") + assert.ElementsMatch(t, uniqueStrings(stdGroups), uniqueStrings(idGroupIDs), "id -G should return same groups as os/user") } func uniqueStrings(ss []string) []string { @@ -343,71 +468,3 @@ func uniqueStrings(ss []string) []string { } return out } - -// TestGetShellFromPasswd_CurrentUser verifies that getShellFromPasswd correctly -// reads the current user's shell from /etc/passwd by comparing it against what -// getent reports (which goes through NSS). -func TestGetShellFromPasswd_CurrentUser(t *testing.T) { - current, err := user.Current() - require.NoError(t, err) - - shell := getShellFromPasswd(current.Uid) - if shell == "" { - t.Skip("current user not found in /etc/passwd (may be an NSS-only user)") - } - - assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) - - if _, err := exec.LookPath("getent"); err == nil { - _, getentShell, getentErr := runGetent(current.Uid) - if getentErr == nil && getentShell != "" { - assert.Equal(t, getentShell, shell, "shell from /etc/passwd should match getent") - } - } -} - -// TestGetShellFromPasswd_RootUser verifies that getShellFromPasswd can read -// root's shell from /etc/passwd. Root is guaranteed to be in /etc/passwd on -// any standard Unix system. -func TestGetShellFromPasswd_RootUser(t *testing.T) { - shell := getShellFromPasswd("0") - require.NotEmpty(t, shell, "root (UID 0) must be in /etc/passwd") - assert.True(t, shell[0] == '/', "root shell should be an absolute path, got %q", shell) -} - -// TestGetShellFromPasswd_NonexistentUID verifies that getShellFromPasswd -// returns empty for a UID that doesn't exist in /etc/passwd. -func TestGetShellFromPasswd_NonexistentUID(t *testing.T) { - shell := getShellFromPasswd("4294967294") - assert.Empty(t, shell, "nonexistent UID should return empty shell") -} - -// TestGetShellFromPasswd_MatchesGetentForKnownUsers reads /etc/passwd directly -// and cross-validates every entry against getent to ensure parseGetentPasswd -// and getShellFromPasswd agree on shell values. -func TestGetShellFromPasswd_MatchesGetentForKnownUsers(t *testing.T) { - if _, err := exec.LookPath("getent"); err != nil { - t.Skip("getent not available") - } - - // Pick a few well-known system UIDs that are virtually always in /etc/passwd. - uids := []string{"0"} // root - - current, err := user.Current() - require.NoError(t, err) - uids = append(uids, current.Uid) - - for _, uid := range uids { - passwdShell := getShellFromPasswd(uid) - if passwdShell == "" { - continue - } - - _, getentShell, err := runGetent(uid) - if err != nil { - continue - } - - assert.Equal(t, getentShell, passwdShell, "shell mismatch for UID %s", uid) - } -} diff --git a/client/internal/getent/windows.go b/client/internal/getent/windows.go new file mode 100644 index 000000000..61881d162 --- /dev/null +++ b/client/internal/getent/windows.go @@ -0,0 +1,36 @@ +//go:build windows + +package getent + +import ( + "errors" + "os/user" +) + +// Windows does not use NSS or getent; os/user resolves accounts there +// without cgo, so everything delegates to it. + +// LookupUser looks up a user by name. +func LookupUser(username string) (*user.User, error) { + return user.Lookup(username) +} + +// LookupUserID looks up a user by UID. +func LookupUserID(uid string) (*user.User, error) { + return user.LookupId(uid) +} + +// CurrentUser returns the user this process runs as. +func CurrentUser() (*user.User, error) { + return user.Current() +} + +// GroupIDs returns the IDs of the groups the user is a member of. +func GroupIDs(u *user.User) ([]string, error) { + return u.GroupIds() +} + +// UserShell is unanswerable on Windows, which has no login-shell database. +func UserShell(string) (string, error) { + return "", errors.ErrUnsupported +} diff --git a/client/ssh/server/getent_cgo_unix.go b/client/ssh/server/getent_cgo_unix.go deleted file mode 100644 index 4afbfc627..000000000 --- a/client/ssh/server/getent_cgo_unix.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build cgo && !osusergo && !windows - -package server - -import "os/user" - -// lookupWithGetent with CGO delegates directly to os/user.Lookup. -// When CGO is enabled, os/user uses libc (getpwnam_r) which goes through -// the NSS stack natively. If it fails, the user truly doesn't exist and -// getent would also fail. -func lookupWithGetent(username string) (*user.User, error) { - return user.Lookup(username) -} - -// currentUserWithGetent with CGO delegates directly to os/user.Current. -func currentUserWithGetent() (*user.User, error) { - return user.Current() -} - -// groupIdsWithFallback with CGO delegates directly to user.GroupIds. -// libc's getgrouplist handles NSS groups natively. -func groupIdsWithFallback(u *user.User) ([]string, error) { - return u.GroupIds() -} diff --git a/client/ssh/server/getent_nocgo_unix.go b/client/ssh/server/getent_nocgo_unix.go deleted file mode 100644 index 314daae4c..000000000 --- a/client/ssh/server/getent_nocgo_unix.go +++ /dev/null @@ -1,74 +0,0 @@ -//go:build (!cgo || osusergo) && !windows - -package server - -import ( - "os" - "os/user" - "strconv" - - log "github.com/sirupsen/logrus" -) - -// lookupWithGetent looks up a user by name, falling back to getent if os/user fails. -// Without CGO, os/user only reads /etc/passwd and misses NSS-provided users. -// getent goes through the host's NSS stack. -func lookupWithGetent(username string) (*user.User, error) { - u, err := user.Lookup(username) - if err == nil { - return u, nil - } - - stdErr := err - log.Debugf("os/user.Lookup(%q) failed, trying getent: %v", username, err) - - u, _, getentErr := runGetent(username) - if getentErr != nil { - log.Debugf("getent fallback for %q also failed: %v", username, getentErr) - return nil, stdErr - } - - return u, nil -} - -// currentUserWithGetent gets the current user, falling back to getent if os/user fails. -func currentUserWithGetent() (*user.User, error) { - u, err := user.Current() - if err == nil { - return u, nil - } - - stdErr := err - uid := strconv.Itoa(os.Getuid()) - log.Debugf("os/user.Current() failed, trying getent with UID %s: %v", uid, err) - - u, _, getentErr := runGetent(uid) - if getentErr != nil { - return nil, stdErr - } - - return u, nil -} - -// groupIdsWithFallback gets group IDs for a user via the id command first, -// falling back to user.GroupIds(). -// NOTE: unlike lookupWithGetent/currentUserWithGetent which try stdlib first, -// this intentionally tries `id -G` first because without CGO, user.GroupIds() -// only reads /etc/group and silently returns incomplete results for NSS users -// (no error, just missing groups). The id command goes through NSS and returns -// the full set. -func groupIdsWithFallback(u *user.User) ([]string, error) { - ids, err := runIdGroups(u.Username) - if err == nil { - return ids, nil - } - - log.Debugf("id -G %q failed, falling back to user.GroupIds(): %v", u.Username, err) - - ids, stdErr := u.GroupIds() - if stdErr != nil { - return nil, stdErr - } - - return ids, nil -} diff --git a/client/ssh/server/getent_unix.go b/client/ssh/server/getent_unix.go deleted file mode 100644 index a3a9641f8..000000000 --- a/client/ssh/server/getent_unix.go +++ /dev/null @@ -1,127 +0,0 @@ -//go:build !windows - -package server - -import ( - "context" - "fmt" - "os/exec" - "os/user" - "runtime" - "strings" - "time" -) - -const getentTimeout = 5 * time.Second - -// getShellFromGetent gets a user's login shell via getent by UID. -// This is needed even with CGO because getShellFromPasswd reads /etc/passwd -// directly and won't find NSS-provided users there. -func getShellFromGetent(userID string) string { - _, shell, err := runGetent(userID) - if err != nil { - return "" - } - return shell -} - -// runGetent executes `getent passwd ` and returns the user and login shell. -func runGetent(query string) (*user.User, string, error) { - if !validateGetentInput(query) { - return nil, "", fmt.Errorf("invalid getent input: %q", query) - } - - ctx, cancel := context.WithTimeout(context.Background(), getentTimeout) - defer cancel() - - out, err := exec.CommandContext(ctx, "getent", "passwd", query).Output() - if err != nil { - return nil, "", fmt.Errorf("getent passwd %s: %w", query, err) - } - - return parseGetentPasswd(string(out)) -} - -// parseGetentPasswd parses getent passwd output: "name:x:uid:gid:gecos:home:shell" -func parseGetentPasswd(output string) (*user.User, string, error) { - fields := strings.SplitN(strings.TrimSpace(output), ":", 8) - if len(fields) < 6 { - return nil, "", fmt.Errorf("unexpected getent output (need 6+ fields): %q", output) - } - - if fields[0] == "" || fields[2] == "" || fields[3] == "" { - return nil, "", fmt.Errorf("missing required fields in getent output: %q", output) - } - - var shell string - if len(fields) >= 7 { - shell = fields[6] - } - - return &user.User{ - Username: fields[0], - Uid: fields[2], - Gid: fields[3], - Name: fields[4], - HomeDir: fields[5], - }, shell, nil -} - -// validateGetentInput checks that the input is safe to pass to getent or id. -// Allows POSIX usernames, numeric UIDs, and common NSS extensions -// (@ for Kerberos, $ for Samba, + for NIS compat). A leading hyphen is -// rejected so the input can never be parsed as a command-line flag. -func validateGetentInput(input string) bool { - maxLen := 32 - if runtime.GOOS == "linux" { - maxLen = 256 - } - - if len(input) == 0 || len(input) > maxLen { - return false - } - - if input[0] == '-' { - return false - } - - for _, r := range input { - if isAllowedGetentChar(r) { - continue - } - return false - } - return true -} - -func isAllowedGetentChar(r rune) bool { - if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' { - return true - } - switch r { - case '.', '_', '-', '@', '+', '$': - return true - } - return false -} - -// runIdGroups runs `id -G ` and returns the space-separated group IDs. -func runIdGroups(username string) ([]string, error) { - if !validateGetentInput(username) { - return nil, fmt.Errorf("invalid username for id command: %q", username) - } - - ctx, cancel := context.WithTimeout(context.Background(), getentTimeout) - defer cancel() - - out, err := exec.CommandContext(ctx, "id", "-G", username).Output() - if err != nil { - return nil, fmt.Errorf("id -G %s: %w", username, err) - } - - trimmed := strings.TrimSpace(string(out)) - if trimmed == "" { - return nil, fmt.Errorf("id -G %s: empty output", username) - } - return strings.Fields(trimmed), nil -} diff --git a/client/ssh/server/getent_windows.go b/client/ssh/server/getent_windows.go deleted file mode 100644 index 3e76b3e8e..000000000 --- a/client/ssh/server/getent_windows.go +++ /dev/null @@ -1,26 +0,0 @@ -//go:build windows - -package server - -import "os/user" - -// lookupWithGetent on Windows just delegates to os/user.Lookup. -// Windows does not use NSS/getent; its user lookup works without CGO. -func lookupWithGetent(username string) (*user.User, error) { - return user.Lookup(username) -} - -// currentUserWithGetent on Windows just delegates to os/user.Current. -func currentUserWithGetent() (*user.User, error) { - return user.Current() -} - -// getShellFromGetent is a no-op on Windows; shell resolution uses PowerShell detection. -func getShellFromGetent(_ string) string { - return "" -} - -// groupIdsWithFallback on Windows just delegates to u.GroupIds(). -func groupIdsWithFallback(u *user.User) ([]string, error) { - return u.GroupIds() -} diff --git a/client/ssh/server/shell.go b/client/ssh/server/shell.go index 1e8ff5e31..7b356b2a0 100644 --- a/client/ssh/server/shell.go +++ b/client/ssh/server/shell.go @@ -13,6 +13,8 @@ import ( "github.com/gliderlabs/ssh" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/getent" ) const ( @@ -56,7 +58,11 @@ func getUnixUserShell(userID string) string { return shell } - if shell := getShellFromGetent(userID); shell != "" { + shell, err := getent.UserShell(userID) + if err != nil { + log.Debugf("look up the shell for uid %s through getent: %v", userID, err) + } + if shell != "" { return shell } diff --git a/client/ssh/server/shell_unix_test.go b/client/ssh/server/shell_unix_test.go new file mode 100644 index 000000000..c5e65e535 --- /dev/null +++ b/client/ssh/server/shell_unix_test.go @@ -0,0 +1,94 @@ +//go:build !windows + +package server + +import ( + "os/exec" + "os/user" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/client/internal/getent" +) + +// TestGetShellFromPasswd_CurrentUser verifies that getShellFromPasswd correctly +// reads the current user's shell from /etc/passwd by comparing it against what +// getent reports (which goes through NSS). +func TestGetShellFromPasswd_CurrentUser(t *testing.T) { + current, err := user.Current() + require.NoError(t, err) + + shell := getShellFromPasswd(current.Uid) + if shell == "" { + t.Skip("current user not found in /etc/passwd (may be an NSS-only user)") + } + + assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) + + if _, err := exec.LookPath("getent"); err == nil { + getentShell, getentErr := getent.UserShell(current.Uid) + if getentErr == nil && getentShell != "" { + assert.Equal(t, getentShell, shell, "shell from /etc/passwd should match getent") + } + } +} + +// TestGetShellFromPasswd_RootUser verifies that getShellFromPasswd can read +// root's shell from /etc/passwd. Root is guaranteed to be in /etc/passwd on +// any standard Unix system. +func TestGetShellFromPasswd_RootUser(t *testing.T) { + shell := getShellFromPasswd("0") + require.NotEmpty(t, shell, "root (UID 0) must be in /etc/passwd") + assert.True(t, shell[0] == '/', "root shell should be an absolute path, got %q", shell) +} + +// TestGetShellFromPasswd_NonexistentUID verifies that getShellFromPasswd +// returns empty for a UID that doesn't exist in /etc/passwd. +func TestGetShellFromPasswd_NonexistentUID(t *testing.T) { + shell := getShellFromPasswd("4294967294") + assert.Empty(t, shell, "nonexistent UID should return empty shell") +} + +// TestGetShellFromPasswd_MatchesGetentForKnownUsers reads /etc/passwd directly +// and cross-validates every entry against getent to ensure the two shell +// sources agree. +func TestGetShellFromPasswd_MatchesGetentForKnownUsers(t *testing.T) { + if _, err := exec.LookPath("getent"); err != nil { + t.Skip("getent not available") + } + + // Pick a few well-known system UIDs that are virtually always in /etc/passwd. + uids := []string{"0"} // root + + current, err := user.Current() + require.NoError(t, err) + uids = append(uids, current.Uid) + + for _, uid := range uids { + passwdShell := getShellFromPasswd(uid) + if passwdShell == "" { + continue + } + + getentShell, err := getent.UserShell(uid) + if err != nil { + continue + } + + assert.Equal(t, getentShell, passwdShell, "shell mismatch for UID %s", uid) + } +} + +// TestIntegration_ShellLookupChain tests the full shell resolution chain +// (getShellFromPasswd -> getent -> $SHELL -> default). +func TestIntegration_ShellLookupChain(t *testing.T) { + current, err := user.Current() + require.NoError(t, err) + + // getUserShell is the top-level function used by the SSH server. + shell := getUserShell(current.Uid) + require.NotEmpty(t, shell, "getUserShell must always return a shell") + assert.True(t, shell[0] == '/', "shell should be an absolute path, got %q", shell) +} diff --git a/client/ssh/server/user_utils.go b/client/ssh/server/user_utils.go index bc2aa2d7d..c04ab8194 100644 --- a/client/ssh/server/user_utils.go +++ b/client/ssh/server/user_utils.go @@ -9,6 +9,8 @@ import ( "strings" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/getent" ) var ( @@ -23,8 +25,8 @@ func isPlatformUnix() bool { // Dependency injection variables for testing - allows mocking dynamic runtime checks var ( - getCurrentUser = currentUserWithGetent - lookupUser = lookupWithGetent + getCurrentUser = getent.CurrentUser + lookupUser = getent.LookupUser getCurrentOS = func() string { return runtime.GOOS } getIsProcessPrivileged = isCurrentProcessPrivileged diff --git a/client/ssh/server/userswitching_unix.go b/client/ssh/server/userswitching_unix.go index 220e2240f..ae60ec64c 100644 --- a/client/ssh/server/userswitching_unix.go +++ b/client/ssh/server/userswitching_unix.go @@ -16,6 +16,8 @@ import ( "github.com/gliderlabs/ssh" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/internal/getent" ) // POSIX portable filename character set regex: [a-zA-Z0-9._-] @@ -160,7 +162,7 @@ func (s *Server) parseUserCredentials(localUser *user.User) (uint32, uint32, []u // getSupplementaryGroups retrieves supplementary group IDs for a user. // Uses id/getent fallback for NSS users in CGO_ENABLED=0 builds. func (s *Server) getSupplementaryGroups(u *user.User) ([]uint32, error) { - groupIDStrings, err := groupIdsWithFallback(u) + groupIDStrings, err := getent.GroupIDs(u) if err != nil { return nil, fmt.Errorf("get group IDs for user %s: %w", u.Username, err) }