diff --git a/client/ssh/server/command_execution_windows.go b/client/ssh/server/command_execution_windows.go index e1ba777f606..feb8daa26c7 100644 --- a/client/ssh/server/command_execution_windows.go +++ b/client/ssh/server/command_execution_windows.go @@ -243,7 +243,7 @@ func (s *Server) setUserEnvironmentVariables(envMap map[string]string, userProfi // prepareCommandEnv prepares environment variables for command execution on Windows func (s *Server) prepareCommandEnv(logger *log.Entry, localUser *user.User, session ssh.Session) []string { - username, domain := s.parseUsername(localUser.Username) + username, domain := parseUsername(localUser.Username) userEnv, err := s.getUserEnvironment(logger, username, domain) if err != nil { log.Debugf("failed to get user environment for %s\\%s, using fallback: %v", domain, username, err) @@ -383,7 +383,7 @@ func (s *Server) executeCommandWithPty(logger *log.Entry, session ssh.Session, _ return false } - username, domain := s.parseUsername(localUser.Username) + username, domain := parseUsername(localUser.Username) shell := getUserShell(localUser.Uid) req := PtyExecutionRequest{ diff --git a/client/ssh/server/privileges_other.go b/client/ssh/server/privileges_other.go new file mode 100644 index 00000000000..5aaa461b083 --- /dev/null +++ b/client/ssh/server/privileges_other.go @@ -0,0 +1,15 @@ +//go:build !windows + +package server + +// isProcessElevated is only meaningful on Windows; other platforms use the +// effective UID check in isCurrentProcessPrivileged. +func isProcessElevated() bool { + return false +} + +// isWindowsAccountPrivileged is only reachable on Windows. Fail closed if it +// is ever called on another platform. +func isWindowsAccountPrivileged(string) bool { + return true +} diff --git a/client/ssh/server/privileges_windows.go b/client/ssh/server/privileges_windows.go new file mode 100644 index 00000000000..6b8d497089d --- /dev/null +++ b/client/ssh/server/privileges_windows.go @@ -0,0 +1,223 @@ +//go:build windows + +package server + +import ( + "fmt" + "strings" + "unsafe" + + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" +) + +var ( + netapi32 = windows.NewLazySystemDLL("netapi32.dll") + procNetUserGetLocalGroups = netapi32.NewProc("NetUserGetLocalGroups") +) + +const ( + // lgIncludeIndirect makes NetUserGetLocalGroups also return local groups + // the user belongs to through a global group. + lgIncludeIndirect = 0x1 + maxPreferredLength = 0xFFFFFFFF +) + +// localGroupUsersInfo0 mirrors LOCALGROUP_USERS_INFO_0. +type localGroupUsersInfo0 struct { + name *uint16 +} + +// isProcessElevated reports whether the current process token is elevated +// (TokenElevation): true for elevated administrators, the built-in +// Administrator, administrators with UAC disabled, and SYSTEM; false for +// standard users and administrators running with a UAC-filtered token. +func isProcessElevated() bool { + return windows.GetCurrentProcessToken().IsElevated() +} + +// isWindowsAccountPrivileged reports whether the account is privileged on this +// machine: a well-known service account, a built-in Administrator (RID 500), +// or a member of the local Administrators group, directly or through nested +// groups. Evaluation errors count as privileged so policy checks fail closed. +func isWindowsAccountPrivileged(username string) bool { + sid, _, _, err := windows.LookupSID("", username) + if err != nil { + log.Warnf("privilege check: SID lookup for %q failed, treating as privileged: %v", username, err) + return true + } + + if isPrivilegedUserSID(sid) { + return true + } + + member, err := isLocalAdminsMember(username) + if err != nil { + log.Warnf("privilege check: cannot determine Administrators membership for %q, treating as privileged: %v", username, err) + return true + } + return member +} + +// isPrivilegedUserSID reports whether the SID itself identifies a privileged +// principal, without consulting group membership. +func isPrivilegedUserSID(sid *windows.SID) bool { + wellKnown := []windows.WELL_KNOWN_SID_TYPE{ + windows.WinLocalSystemSid, + windows.WinLocalServiceSid, + windows.WinNetworkServiceSid, + windows.WinBuiltinAdministratorsSid, + } + for _, sidType := range wellKnown { + if sid.IsWellKnown(sidType) { + return true + } + } + return isBuiltinAdministratorSID(sid) +} + +// isBuiltinAdministratorSID reports whether the SID is a machine or domain +// built-in Administrator account (S-1-5-21-...-500). RID 500 is reserved for +// that account; it can be renamed but cannot be removed from the +// Administrators group. +func isBuiltinAdministratorSID(sid *windows.SID) bool { + if sid.IdentifierAuthority() != windows.SECURITY_NT_AUTHORITY { + return false + } + count := sid.SubAuthorityCount() + if count < 2 || sid.SubAuthority(0) != 21 { + return false + } + return sid.SubAuthority(uint32(count-1)) == 500 +} + +// isLocalAdminsMember reports whether the account is a member of the local +// Administrators group. +// +// Local accounts are checked against the local SAM, which is authoritative for +// them and, unlike a token, cannot under-report: UAC filters the tokens of +// local administrators, and a filtered token carries Administrators as +// deny-only, which a membership check on the token would read as "not a +// member". Domain accounts are exempt from that filtering, so for them an S4U +// token is preferred because its group list is LSA's transitive expansion and +// therefore covers nested and universal groups plus the machine's own local +// groups. NetUserGetLocalGroups expands only one global-group hop but needs no +// logon, so it serves as the fallback when no token can be obtained. +func isLocalAdminsMember(username string) (bool, error) { + adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid) + if err != nil { + return false, fmt.Errorf("create Administrators SID: %w", err) + } + + account, domain := parseUsername(username) + if NewPrivilegeDropper().isLocalUser(domain) { + return localGroupsContainSID(account, adminSid) + } + + member, s4uErr := s4uTokenIsMember(account, domain, adminSid) + if s4uErr == nil { + return member, nil + } + log.Debugf("privilege check: S4U membership check for %q failed, falling back to local group enumeration: %v", username, s4uErr) + + member, err = localGroupsContainSID(buildUserCpn(account, domain), adminSid) + if err != nil { + return false, fmt.Errorf("S4U check: %w; local group enumeration: %w", s4uErr, err) + } + return member, nil +} + +// s4uTokenIsMember obtains an S4U token for the account and checks whether the +// given SID is enabled in it. +func s4uTokenIsMember(account, domain string, sid *windows.SID) (bool, error) { + token, err := generateS4UUserToken(log.NewEntry(log.StandardLogger()), account, domain) + if err != nil { + return false, err + } + defer func() { + if err := windows.CloseHandle(token); err != nil { + log.Debugf("close S4U token: %v", err) + } + }() + return windows.Token(token).IsMember(sid) +} + +// localGroupsContainSID reports whether the wanted group is among the local +// groups the account belongs to, directly or through a global group. +// +// The wanted SID is resolved to its group name once and compared against the +// enumerated names. Well-known SIDs resolve from a static table, so that lookup +// needs no domain controller, and it keeps the comparison correct for a renamed +// or localized group because both sides then carry the new name. Resolving each +// enumerated name back to a SID instead would add a lookup per group that can +// block until it times out while a domain controller is unreachable, and cannot +// change the outcome: the names enumerated here are local groups of this +// machine, whose names are unique, so a name match identifies the group. +// +// A failure to resolve the wanted SID is returned rather than reported as +// "not a member", so a privilege check built on this fails closed. +func localGroupsContainSID(username string, want *windows.SID) (bool, error) { + wantName, _, _, err := want.LookupAccount("") + if err != nil { + return false, fmt.Errorf("resolve group SID %s to a name: %w", want, err) + } + + groups, err := netUserGetLocalGroups(username) + if err != nil { + return false, err + } + + for _, group := range groups { + if strings.EqualFold(group, wantName) { + return true, nil + } + } + return false, nil +} + +// netUserGetLocalGroups returns the names of the local groups the account is a +// member of, including indirect membership through global groups. +func netUserGetLocalGroups(username string) ([]string, error) { + name16, err := windows.UTF16PtrFromString(username) + if err != nil { + return nil, fmt.Errorf("convert username: %w", err) + } + + var buf *byte + var entriesRead, totalEntries uint32 + status, _, _ := procNetUserGetLocalGroups.Call( + 0, // local server + uintptr(unsafe.Pointer(name16)), + 0, // level 0: LOCALGROUP_USERS_INFO_0 + lgIncludeIndirect, + uintptr(unsafe.Pointer(&buf)), + maxPreferredLength, + uintptr(unsafe.Pointer(&entriesRead)), + uintptr(unsafe.Pointer(&totalEntries)), + ) + if status != 0 { + return nil, fmt.Errorf("NetUserGetLocalGroups for %q: status %d", username, status) + } + if buf == nil { + return nil, nil + } + defer func() { + if err := windows.NetApiBufferFree(buf); err != nil { + log.Debugf("free NetApi buffer: %v", err) + } + }() + + // MAX_PREFERRED_LENGTH makes the API allocate as much as it needs, so a + // short read is not expected. Report it rather than silently returning a + // subset of the account's groups. + if entriesRead != totalEntries { + return nil, fmt.Errorf("NetUserGetLocalGroups for %q returned %d of %d groups", username, entriesRead, totalEntries) + } + + entries := unsafe.Slice((*localGroupUsersInfo0)(unsafe.Pointer(buf)), entriesRead) + groups := make([]string, 0, entriesRead) + for _, entry := range entries { + groups = append(groups, windows.UTF16PtrToString(entry.name)) + } + return groups, nil +} diff --git a/client/ssh/server/privileges_windows_test.go b/client/ssh/server/privileges_windows_test.go new file mode 100644 index 00000000000..406ce613758 --- /dev/null +++ b/client/ssh/server/privileges_windows_test.go @@ -0,0 +1,293 @@ +//go:build windows + +package server + +import ( + "os/user" + "testing" + "unsafe" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" +) + +// filterNormalAccount limits NetUserEnum to normal user accounts. +const filterNormalAccount = 0x2 + +// TOKEN_ELEVATION_TYPE values. +const ( + tokenElevationTypeDefault = 1 + tokenElevationTypeFull = 2 + tokenElevationTypeLimited = 3 +) + +// tokenElevationType reads TokenElevationType from a token. +func tokenElevationType(token windows.Token) (uint32, error) { + var elevationType, returnedLen uint32 + err := windows.GetTokenInformation(token, windows.TokenElevationType, + (*byte)(unsafe.Pointer(&elevationType)), uint32(unsafe.Sizeof(elevationType)), &returnedLen) + if err != nil { + return 0, err + } + return elevationType, nil +} + +// userInfo0 mirrors USER_INFO_0. +type userInfo0 struct { + name *uint16 +} + +func mustParseSID(t *testing.T, s string) *windows.SID { + t.Helper() + sid, err := windows.StringToSid(s) + require.NoError(t, err, "parse SID %s", s) + return sid +} + +// localAccountNames returns the names of the local user accounts. +func localAccountNames(t *testing.T) []string { + t.Helper() + + var buf *byte + var entriesRead, totalEntries, resume uint32 + err := windows.NetUserEnum(nil, 0, filterNormalAccount, &buf, maxPreferredLength, + &entriesRead, &totalEntries, &resume) + require.NoError(t, err, "enumerate local users") + t.Cleanup(func() { + require.NoError(t, windows.NetApiBufferFree(buf), "free NetApi buffer") + }) + + entries := unsafe.Slice((*userInfo0)(unsafe.Pointer(buf)), entriesRead) + names := make([]string, 0, entriesRead) + for _, entry := range entries { + names = append(names, windows.UTF16PtrToString(entry.name)) + } + return names +} + +// localAccountNameByRID returns the name of the local account carrying the +// given RID. Accounts such as Administrator and Guest can be renamed and are +// localized, so tests must not name them literally. +func localAccountNameByRID(t *testing.T, rid uint32) string { + t.Helper() + + for _, name := range localAccountNames(t) { + sid, _, _, err := windows.LookupSID("", name) + if err != nil { + continue + } + if sid.IdentifierAuthority() != windows.SECURITY_NT_AUTHORITY { + continue + } + count := sid.SubAuthorityCount() + if count < 2 || sid.SubAuthority(0) != 21 { + continue + } + if sid.SubAuthority(uint32(count-1)) == rid { + return name + } + } + + t.Fatalf("no local account with RID %d", rid) + return "" +} + +// wellKnownAccountName resolves a well-known SID to the qualified account name +// the local system uses for it, which is localized. +func wellKnownAccountName(t *testing.T, sidType windows.WELL_KNOWN_SID_TYPE) string { + t.Helper() + + sid, err := windows.CreateWellKnownSid(sidType) + require.NoError(t, err, "create well-known SID") + name, domain, _, err := sid.LookupAccount("") + require.NoError(t, err, "resolve %s to an account name", sid) + if domain == "" { + return name + } + return domain + `\` + name +} + +func TestIsBuiltinAdministratorSID(t *testing.T) { + tests := []struct { + name string + sid string + want bool + }{ + {"machine_administrator", "S-1-5-21-1111111111-2222222222-3333333333-500", true}, + {"domain_administrator", "S-1-5-21-3390233681-4087452608-412898826-500", true}, + {"regular_user", "S-1-5-21-1111111111-2222222222-3333333333-1001", false}, + {"guest_account", "S-1-5-21-1111111111-2222222222-3333333333-501", false}, + {"domain_admins_group", "S-1-5-21-1111111111-2222222222-3333333333-512", false}, + {"system", "S-1-5-18", false}, + {"administrators_group", "S-1-5-32-544", false}, + {"non_nt_authority", "S-1-1-0", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isBuiltinAdministratorSID(mustParseSID(t, tt.sid)) + assert.Equal(t, tt.want, result, "RID 500 detection for %s", tt.sid) + }) + } +} + +func TestIsPrivilegedUserSID(t *testing.T) { + tests := []struct { + name string + sid string + want bool + }{ + {"local_system", "S-1-5-18", true}, + {"local_service", "S-1-5-19", true}, + {"network_service", "S-1-5-20", true}, + {"administrators_group", "S-1-5-32-544", true}, + {"builtin_administrator", "S-1-5-21-1111111111-2222222222-3333333333-500", true}, + {"regular_user", "S-1-5-21-1111111111-2222222222-3333333333-1001", false}, + {"users_group", "S-1-5-32-545", false}, + {"everyone", "S-1-1-0", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isPrivilegedUserSID(mustParseSID(t, tt.sid)) + assert.Equal(t, tt.want, result, "SID privilege classification for %s", tt.sid) + }) + } +} + +func TestIsWindowsAccountPrivileged(t *testing.T) { + tests := []struct { + name string + username string + want bool + }{ + {"system", wellKnownAccountName(t, windows.WinLocalSystemSid), true}, + {"local_service", wellKnownAccountName(t, windows.WinLocalServiceSid), true}, + {"network_service", wellKnownAccountName(t, windows.WinNetworkServiceSid), true}, + {"administrators_group", wellKnownAccountName(t, windows.WinBuiltinAdministratorsSid), true}, + // The built-in Administrator (RID 500) and Guest (RID 501) accounts + // exist on every Windows installation, though they may be disabled. + {"builtin_administrator", localAccountNameByRID(t, 500), true}, + {"guest", localAccountNameByRID(t, 501), false}, + // Unresolvable accounts fail closed. + {"nonexistent_user", "netbird-no-such-user", true}, + {"empty_username", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isWindowsAccountPrivileged(tt.username) + assert.Equal(t, tt.want, result, "account privilege classification for %q", tt.username) + }) + } +} + +func TestIsProcessElevated(t *testing.T) { + elevated := isProcessElevated() + + // TokenElevationType is a second, independent view of the same token: + // Full means elevated and Limited means a filtered administrator, while + // Default covers both a standard user and an administrator with no linked + // token (UAC off, the built-in Administrator, SYSTEM), so it implies nothing. + elevationType, err := tokenElevationType(windows.GetCurrentProcessToken()) + require.NoError(t, err, "read token elevation type") + + adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid) + require.NoError(t, err, "create Administrators SID") + + // Token(0) makes CheckTokenMembership evaluate the caller's own token. It + // counts only enabled SIDs, so a filtered administrator reports false here. + member, err := windows.Token(0).IsMember(adminSid) + require.NoError(t, err, "check own Administrators membership") + + t.Logf("elevated=%v elevationType=%d memberOfAdministrators=%v", elevated, elevationType, member) + + switch elevationType { + case tokenElevationTypeFull: + assert.True(t, elevated, "a token of elevation type Full must report elevated") + case tokenElevationTypeLimited: + assert.False(t, elevated, "a filtered administrator token must not report elevated") + } + + // Administrators enabled in the token means the token wields administrative + // rights, which is what elevation reports. + if member { + assert.True(t, elevated, "token with enabled Administrators membership must report elevated") + } +} + +// TestS4UMembershipAgreesWithLocalGroups exercises the S4U token path used +// for domain accounts. S4U logons need the TCB privilege, so the test runs +// only as SYSTEM (which is how CI executes the suite). For local accounts the +// token's Administrators membership must agree with the SAM enumeration. +func TestS4UMembershipAgreesWithLocalGroups(t *testing.T) { + system, err := windows.CreateWellKnownSid(windows.WinLocalSystemSid) + require.NoError(t, err, "create SYSTEM SID") + current, err := user.Current() + require.NoError(t, err, "get current user") + if current.Uid != system.String() { + t.Skipf("S4U logon requires SYSTEM (running as %s)", current.Username) + } + + adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid) + require.NoError(t, err, "create Administrators SID") + + checked := 0 + for _, name := range localAccountNames(t) { + viaToken, err := s4uTokenIsMember(name, ".", adminSid) + if err != nil { + // Disabled or logon-restricted accounts cannot get an S4U logon. + t.Logf("skipping %s: %v", name, err) + continue + } + viaSAM, err := localGroupsContainSID(name, adminSid) + require.NoError(t, err, "enumerate local groups for %s", name) + + assert.Equal(t, viaSAM, viaToken, "S4U token and SAM enumeration must agree on Administrators membership for %s", name) + checked++ + } + // Ineligible accounts are skipped, so without this the test could report + // success while comparing nothing at all. + require.Positive(t, checked, "no local account completed an S4U logon, so nothing was compared") + t.Logf("checked %d local accounts via S4U", checked) +} + +// TestLocalGroupsContainSID_Administrator checks the positive case against the +// built-in Administrator, a member of Administrators on every installation. +func TestLocalGroupsContainSID_Administrator(t *testing.T) { + adminSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid) + require.NoError(t, err, "create Administrators SID") + + administrator := localAccountNameByRID(t, 500) + member, err := localGroupsContainSID(administrator, adminSid) + require.NoError(t, err, "enumerate local groups for %s", administrator) + assert.True(t, member, "%s is a member of the Administrators group", administrator) +} + +// TestLocalGroupsContainSID_UnresolvableGroupFailsClosed covers a wanted SID +// that resolves to no group: the error must surface rather than being reported +// as "not a member", so the privilege check treats the account as privileged. +func TestLocalGroupsContainSID_UnresolvableGroupFailsClosed(t *testing.T) { + unknown := mustParseSID(t, "S-1-5-21-1111111111-2222222222-3333333333-4444") + + _, err := localGroupsContainSID(localAccountNameByRID(t, 500), unknown) + require.Error(t, err, "must report an error when the wanted group cannot be identified") +} + +func TestLocalGroupsContainSID_Guest(t *testing.T) { + guestsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinGuestsSid) + require.NoError(t, err, "create Guests SID") + adminsSid, err := windows.CreateWellKnownSid(windows.WinBuiltinAdministratorsSid) + require.NoError(t, err, "create Administrators SID") + + guest := localAccountNameByRID(t, 501) + + inGuests, err := localGroupsContainSID(guest, guestsSid) + require.NoError(t, err, "enumerate local groups for %s", guest) + assert.True(t, inGuests, "%s is a member of the Guests group", guest) + + inAdmins, err := localGroupsContainSID(guest, adminsSid) + require.NoError(t, err, "enumerate local groups for %s", guest) + assert.False(t, inAdmins, "%s is not a member of the Administrators group", guest) +} diff --git a/client/ssh/server/server_config_test.go b/client/ssh/server/server_config_test.go index f70e29963dd..0717c238d25 100644 --- a/client/ssh/server/server_config_test.go +++ b/client/ssh/server/server_config_test.go @@ -420,6 +420,13 @@ func TestServer_PortConflictHandling(t *testing.T) { func TestServer_IsPrivilegedUser(t *testing.T) { + // Windows classification depends on account SIDs and group membership, and + // the accounts involved carry localized, renameable names. It is covered by + // TestIsWindowsAccountPrivileged, which resolves them from well-known SIDs. + if runtime.GOOS == "windows" { + t.Skip("covered by TestIsWindowsAccountPrivileged") + } + tests := []struct { username string expected bool @@ -440,39 +447,11 @@ func TestServer_IsPrivilegedUser(t *testing.T) { expected: false, description: "empty username should not be privileged", }, - } - - // Add Windows-specific tests - if runtime.GOOS == "windows" { - tests = append(tests, []struct { - username string - expected bool - description string - }{ - { - username: "Administrator", - expected: true, - description: "Administrator should be considered privileged on Windows", - }, - { - username: "administrator", - expected: true, - description: "administrator should be considered privileged on Windows (case insensitive)", - }, - }...) - } else { - // On non-Windows systems, Administrator should not be privileged - tests = append(tests, []struct { - username string - expected bool - description string - }{ - { - username: "Administrator", - expected: false, - description: "Administrator should not be privileged on non-Windows systems", - }, - }...) + { + username: "Administrator", + expected: false, + description: "Administrator should not be privileged on non-Windows systems", + }, } for _, tt := range tests { diff --git a/client/ssh/server/sftp_windows.go b/client/ssh/server/sftp_windows.go index dc532b9e766..25cd1729899 100644 --- a/client/ssh/server/sftp_windows.go +++ b/client/ssh/server/sftp_windows.go @@ -17,7 +17,7 @@ import ( // createSftpCommand creates a Windows SFTP command with user switching. // The caller must close the returned token handle after starting the process. func (s *Server) createSftpCommand(targetUser *user.User, sess ssh.Session) (*exec.Cmd, windows.Token, error) { - username, domain := s.parseUsername(targetUser.Username) + username, domain := parseUsername(targetUser.Username) netbirdPath, err := os.Executable() if err != nil { diff --git a/client/ssh/server/user_utils.go b/client/ssh/server/user_utils.go index bc2aa2d7d82..93a86fd1829 100644 --- a/client/ssh/server/user_utils.go +++ b/client/ssh/server/user_utils.go @@ -16,11 +16,6 @@ var ( ErrPrivilegedUserSwitch = errors.New("cannot switch to privileged user - current user lacks required privileges") ) -// isPlatformUnix returns true for Unix-like platforms (Linux, macOS, etc.) -func isPlatformUnix() bool { - return getCurrentOS() != "windows" -} - // Dependency injection variables for testing - allows mocking dynamic runtime checks var ( getCurrentUser = currentUserWithGetent @@ -29,6 +24,9 @@ var ( getIsProcessPrivileged = isCurrentProcessPrivileged getEuid = os.Geteuid + + getProcessElevated = isProcessElevated + getWindowsAccountPrivileged = isWindowsAccountPrivileged ) const ( @@ -65,6 +63,13 @@ type PrivilegeCheckResult struct { RequiresUserSwitching bool } +// privilegeCheckContext holds all context needed for privilege checking +type privilegeCheckContext struct { + currentUser *user.User + currentUserPrivileged bool + allowRoot bool +} + // CheckPrivileges performs comprehensive privilege checking for all SSH features. // This is the single source of truth for privilege decisions across the SSH server. func (s *Server) CheckPrivileges(req PrivilegeCheckRequest) PrivilegeCheckResult { @@ -175,6 +180,42 @@ func (s *Server) resolveRequestedUser(requestedUsername string) (*user.User, err return u, nil } +// SetAllowRootLogin configures root login access +func (s *Server) SetAllowRootLogin(allow bool) { + s.mu.Lock() + defer s.mu.Unlock() + s.allowRootLogin = allow +} + +// userNameLookup performs user lookup with root login permission check +func (s *Server) userNameLookup(username string) (*user.User, error) { + result, err := s.userPrivilegeCheck(username) + if err != nil { + return nil, err + } + return result.User, nil +} + +// userPrivilegeCheck performs user lookup with full privilege check result +func (s *Server) userPrivilegeCheck(username string) (PrivilegeCheckResult, error) { + result := s.CheckPrivileges(PrivilegeCheckRequest{ + RequestedUsername: username, + FeatureSupportsUserSwitch: true, + FeatureName: FeatureSSHLogin, + }) + + if !result.Allowed { + return result, result.Error + } + + return result, nil +} + +// isPlatformUnix returns true for Unix-like platforms (Linux, macOS, etc.) +func isPlatformUnix() bool { + return getCurrentOS() != "windows" +} + // isSameResolvedUser compares two resolved user identities func isSameResolvedUser(user1, user2 *user.User) bool { if user1 == nil || user2 == nil { @@ -183,13 +224,6 @@ func isSameResolvedUser(user1, user2 *user.User) bool { return user1.Uid == user2.Uid } -// privilegeCheckContext holds all context needed for privilege checking -type privilegeCheckContext struct { - currentUser *user.User - currentUserPrivileged bool - allowRoot bool -} - // isSameUser checks if two usernames refer to the same user // SECURITY: This function must be conservative - it should only return true // when we're certain both usernames refer to the exact same user identity @@ -253,159 +287,24 @@ func isWindowsSameUser(requestedUsername, currentUsername string) bool { return strings.EqualFold(reqDomain, curDomain) } -// SetAllowRootLogin configures root login access -func (s *Server) SetAllowRootLogin(allow bool) { - s.mu.Lock() - defer s.mu.Unlock() - s.allowRootLogin = allow -} - -// userNameLookup performs user lookup with root login permission check -func (s *Server) userNameLookup(username string) (*user.User, error) { - result := s.CheckPrivileges(PrivilegeCheckRequest{ - RequestedUsername: username, - FeatureSupportsUserSwitch: true, - FeatureName: FeatureSSHLogin, - }) - - if !result.Allowed { - return nil, result.Error - } - - return result.User, nil -} - -// userPrivilegeCheck performs user lookup with full privilege check result -func (s *Server) userPrivilegeCheck(username string) (PrivilegeCheckResult, error) { - result := s.CheckPrivileges(PrivilegeCheckRequest{ - RequestedUsername: username, - FeatureSupportsUserSwitch: true, - FeatureName: FeatureSSHLogin, - }) - - if !result.Allowed { - return result, result.Error - } - - return result, nil -} - // isPrivilegedUsername checks if the given username represents a privileged user across platforms. -// On Unix: root -// On Windows: Administrator, SYSTEM (case-insensitive) -// Handles domain-qualified usernames like "DOMAIN\Administrator" or "user@domain.com" +// On Unix: root. +// On Windows: well-known service accounts, built-in Administrator accounts, +// and members of the local Administrators group; handles domain-qualified +// usernames like "DOMAIN\user" or "user@domain.com". func isPrivilegedUsername(username string) bool { if getCurrentOS() != "windows" { return username == "root" } - - bareUsername := username - // Handle Windows domain format: DOMAIN\username - if idx := strings.LastIndex(username, `\`); idx != -1 { - bareUsername = username[idx+1:] - } - // Handle email-style format: username@domain.com - if idx := strings.Index(bareUsername, "@"); idx != -1 { - bareUsername = bareUsername[:idx] - } - - return isWindowsPrivilegedUser(bareUsername) -} - -// isWindowsPrivilegedUser checks if a bare username (domain already stripped) represents a Windows privileged account -func isWindowsPrivilegedUser(bareUsername string) bool { - // common privileged usernames (case insensitive) - privilegedNames := []string{ - "administrator", - "admin", - "root", - "system", - "localsystem", - "networkservice", - "localservice", - } - - usernameLower := strings.ToLower(bareUsername) - for _, privilegedName := range privilegedNames { - if usernameLower == privilegedName { - return true - } - } - - // computer accounts (ending with $) are not privileged by themselves - // They only gain privileges through group membership or specific SIDs - - if targetUser, err := lookupUser(bareUsername); err == nil { - return isWindowsPrivilegedSID(targetUser.Uid) - } - - return false -} - -// isWindowsPrivilegedSID checks if a Windows SID represents a privileged account -func isWindowsPrivilegedSID(sid string) bool { - privilegedSIDs := []string{ - "S-1-5-18", // Local System (SYSTEM) - "S-1-5-19", // Local Service (NT AUTHORITY\LOCAL SERVICE) - "S-1-5-20", // Network Service (NT AUTHORITY\NETWORK SERVICE) - "S-1-5-32-544", // Administrators group (BUILTIN\Administrators) - "S-1-5-500", // Built-in Administrator account (local machine RID 500) - } - - for _, privilegedSID := range privilegedSIDs { - if sid == privilegedSID { - return true - } - } - - // Check for domain administrator accounts (RID 500 in any domain) - // Format: S-1-5-21-domain-domain-domain-500 - // This is reliable as RID 500 is reserved for the domain Administrator account - if strings.HasPrefix(sid, "S-1-5-21-") && strings.HasSuffix(sid, "-500") { - return true - } - - // Check for other well-known privileged RIDs in domain contexts - // RID 512 = Domain Admins group, RID 516 = Domain Controllers group - if strings.HasPrefix(sid, "S-1-5-21-") { - if strings.HasSuffix(sid, "-512") || // Domain Admins group - strings.HasSuffix(sid, "-516") || // Domain Controllers group - strings.HasSuffix(sid, "-519") { // Enterprise Admins group - return true - } - } - - return false + return getWindowsAccountPrivileged(username) } // isCurrentProcessPrivileged checks if the current process is running with elevated privileges. // On Unix systems, this means running as root (UID 0). -// On Windows, this means running as Administrator or SYSTEM. +// On Windows, this means the process token is elevated (administrators, SYSTEM). func isCurrentProcessPrivileged() bool { if getCurrentOS() == "windows" { - return isWindowsElevated() + return getProcessElevated() } return getEuid() == 0 } - -// isWindowsElevated checks if the current process is running with elevated privileges on Windows -func isWindowsElevated() bool { - currentUser, err := getCurrentUser() - if err != nil { - log.Errorf("failed to get current user for privilege check, assuming non-privileged: %v", err) - return false - } - - if isWindowsPrivilegedSID(currentUser.Uid) { - log.Debugf("Windows user switching supported: running as privileged SID %s", currentUser.Uid) - return true - } - - if isPrivilegedUsername(currentUser.Username) { - log.Debugf("Windows user switching supported: running as privileged username %s", currentUser.Username) - return true - } - - log.Debugf("Windows user switching not supported: not running as privileged user (current: %s)", currentUser.Uid) - return false -} diff --git a/client/ssh/server/user_utils_test.go b/client/ssh/server/user_utils_test.go index 637dc10d0a6..c734b194713 100644 --- a/client/ssh/server/user_utils_test.go +++ b/client/ssh/server/user_utils_test.go @@ -4,6 +4,7 @@ import ( "errors" "os/user" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -27,8 +28,8 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri originalLookupUser := lookupUser originalGetCurrentOS := getCurrentOS originalGetEuid := getEuid - - // Reset caches to ensure clean test state + originalGetProcessElevated := getProcessElevated + originalGetWindowsAccountPrivileged := getWindowsAccountPrivileged // Set test values - inject platform dependencies getCurrentUser = func() (*user.User, error) { @@ -53,16 +54,31 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri return euid } - // Mock privilege detection based on the test user - getIsProcessPrivileged = func() bool { + // Simulate the Windows token elevation check based on the fixture user: + // the built-in Administrator (RID 500) and SYSTEM run elevated. + getProcessElevated = func() bool { if currentUser == nil { return false } - // Check both username and SID for Windows systems - if os == "windows" && isWindowsPrivilegedSID(currentUser.Uid) { + return currentUser.Uid == "S-1-5-18" || strings.HasSuffix(currentUser.Uid, "-500") + } + + // Simulate the Windows account classifier for the fixture accounts. + // "root" does not exist on Windows; the real classifier fails closed on + // unresolvable accounts, so it counts as privileged here too. + getWindowsAccountPrivileged = func(username string) bool { + bare := username + if idx := strings.LastIndex(bare, `\`); idx != -1 { + bare = bare[idx+1:] + } + if idx := strings.Index(bare, "@"); idx != -1 { + bare = bare[:idx] + } + switch strings.ToLower(bare) { + case "administrator", "system", "root": return true } - return isPrivilegedUsername(currentUser.Username) + return false } // Return cleanup function @@ -71,10 +87,8 @@ func setupTestDependencies(currentUser *user.User, currentUserErr error, os stri lookupUser = originalLookupUser getCurrentOS = originalGetCurrentOS getEuid = originalGetEuid - - getIsProcessPrivileged = isCurrentProcessPrivileged - - // Reset caches after test + getProcessElevated = originalGetProcessElevated + getWindowsAccountPrivileged = originalGetWindowsAccountPrivileged } } @@ -421,6 +435,9 @@ func TestUsedFallback_MeansNoPrivilegeDropping(t *testing.T) { } func TestPrivilegedUsernameDetection(t *testing.T) { + // Windows classification is syscall-backed (SID resolution, group + // membership) and is covered by privileges_windows_test.go; here only the + // Unix logic and the platform dispatch are exercised. tests := []struct { name string username string @@ -432,25 +449,9 @@ func TestPrivilegedUsernameDetection(t *testing.T) { {"unix_regular_user", "alice", "linux", false}, {"unix_root_capital", "Root", "linux", false}, // Case-sensitive - // Windows tests + // Windows dispatch to the (mocked) account classifier {"windows_administrator", "Administrator", "windows", true}, - {"windows_system", "SYSTEM", "windows", true}, - {"windows_admin", "admin", "windows", true}, - {"windows_admin_lowercase", "administrator", "windows", true}, // Case-insensitive - {"windows_domain_admin", "DOMAIN\\Administrator", "windows", true}, - {"windows_email_admin", "admin@domain.com", "windows", true}, {"windows_regular_user", "alice", "windows", false}, - {"windows_domain_user", "DOMAIN\\alice", "windows", false}, - {"windows_localsystem", "localsystem", "windows", true}, - {"windows_networkservice", "networkservice", "windows", true}, - {"windows_localservice", "localservice", "windows", true}, - - // Computer accounts (these depend on current user context in real implementation) - {"windows_computer_account", "WIN2K19-C2$", "windows", false}, // Computer account by itself not privileged - {"windows_domain_computer", "DOMAIN\\COMPUTER$", "windows", false}, // Domain computer account - - // Cross-platform - {"root_on_windows", "root", "windows", true}, // Root should be privileged everywhere } for _, tt := range tests { @@ -460,49 +461,7 @@ func TestPrivilegedUsernameDetection(t *testing.T) { defer cleanup() result := isPrivilegedUsername(tt.username) - assert.Equal(t, tt.privileged, result) - }) - } -} - -func TestWindowsPrivilegedSIDDetection(t *testing.T) { - tests := []struct { - name string - sid string - privileged bool - description string - }{ - // Well-known system accounts - {"system_account", "S-1-5-18", true, "Local System (SYSTEM)"}, - {"local_service", "S-1-5-19", true, "Local Service"}, - {"network_service", "S-1-5-20", true, "Network Service"}, - {"administrators_group", "S-1-5-32-544", true, "Administrators group"}, - {"builtin_administrator", "S-1-5-500", true, "Built-in Administrator"}, - - // Domain accounts - {"domain_administrator", "S-1-5-21-1234567890-1234567890-1234567890-500", true, "Domain Administrator (RID 500)"}, - {"domain_admins_group", "S-1-5-21-1234567890-1234567890-1234567890-512", true, "Domain Admins group"}, - {"domain_controllers_group", "S-1-5-21-1234567890-1234567890-1234567890-516", true, "Domain Controllers group"}, - {"enterprise_admins_group", "S-1-5-21-1234567890-1234567890-1234567890-519", true, "Enterprise Admins group"}, - - // Regular users - {"regular_user", "S-1-5-21-1234567890-1234567890-1234567890-1001", false, "Regular domain user"}, - {"another_regular_user", "S-1-5-21-1234567890-1234567890-1234567890-1234", false, "Another regular user"}, - {"local_user", "S-1-5-21-1234567890-1234567890-1234567890-1000", false, "Local regular user"}, - - // Groups that are not privileged - {"domain_users", "S-1-5-21-1234567890-1234567890-1234567890-513", false, "Domain Users group"}, - {"power_users", "S-1-5-32-547", false, "Power Users group"}, - - // Invalid SIDs - {"malformed_sid", "S-1-5-invalid", false, "Malformed SID"}, - {"empty_sid", "", false, "Empty SID"}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := isWindowsPrivilegedSID(tt.sid) - assert.Equal(t, tt.privileged, result, "Failed for %s: %s", tt.description, tt.sid) + assert.Equal(t, tt.privileged, result, "privilege classification for %s on %s", tt.username, tt.platform) }) } } diff --git a/client/ssh/server/userswitching_windows.go b/client/ssh/server/userswitching_windows.go index 260e1301ed6..9e8cd5b3023 100644 --- a/client/ssh/server/userswitching_windows.go +++ b/client/ssh/server/userswitching_windows.go @@ -91,7 +91,7 @@ func validateUsernameFormat(username string) error { func (s *Server) createExecutorCommand(logger *log.Entry, session ssh.Session, localUser *user.User, hasPty bool) (*exec.Cmd, func(), error) { logger.Debugf("creating Windows executor command for user %s (Pty: %v)", localUser.Username, hasPty) - username, _ := s.parseUsername(localUser.Username) + username, _ := parseUsername(localUser.Username) if err := validateUsername(username); err != nil { return nil, nil, fmt.Errorf("invalid username %q: %w", username, err) } @@ -102,7 +102,7 @@ func (s *Server) createExecutorCommand(logger *log.Entry, session ssh.Session, l // createUserSwitchCommand creates a command with Windows user switching. // Returns the command and a cleanup function that must be called after starting the process. func (s *Server) createUserSwitchCommand(logger *log.Entry, session ssh.Session, localUser *user.User) (*exec.Cmd, func(), error) { - username, domain := s.parseUsername(localUser.Username) + username, domain := parseUsername(localUser.Username) shell := getUserShell(localUser.Uid) @@ -138,7 +138,7 @@ func (s *Server) createUserSwitchCommand(logger *log.Entry, session ssh.Session, } // parseUsername extracts username and domain from a Windows username -func (s *Server) parseUsername(fullUsername string) (username, domain string) { +func parseUsername(fullUsername string) (username, domain string) { // Handle DOMAIN\username format if idx := strings.LastIndex(fullUsername, `\`); idx != -1 { domain = fullUsername[:idx]