From e2ee1dd002af81f1989f630b20af684cc5667289 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:21:01 +0900 Subject: [PATCH] Apply the SSH root-login setting to uid 0, not only to the account named root (#8390) --- client/ssh/server/server_config_test.go | 2 +- client/ssh/server/user_utils.go | 23 ++++++++----- client/ssh/server/user_utils_test.go | 46 +++++++++++++++++++++---- 3 files changed, 55 insertions(+), 16 deletions(-) diff --git a/client/ssh/server/server_config_test.go b/client/ssh/server/server_config_test.go index 983bd7a43..f8ff5416b 100644 --- a/client/ssh/server/server_config_test.go +++ b/client/ssh/server/server_config_test.go @@ -483,7 +483,7 @@ func TestServer_IsPrivilegedUser(t *testing.T) { for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { - result := isPrivilegedOrUnknown(tt.username) + result := isPrivilegedOrUnknown(&user.User{Username: tt.username, Uid: "1000"}) assert.Equal(t, tt.expected, result, tt.description) }) } diff --git a/client/ssh/server/user_utils.go b/client/ssh/server/user_utils.go index f2f33b3d7..6415c21ae 100644 --- a/client/ssh/server/user_utils.go +++ b/client/ssh/server/user_utils.go @@ -6,6 +6,7 @@ import ( "os" "os/user" "runtime" + "strconv" "strings" log "github.com/sirupsen/logrus" @@ -82,7 +83,7 @@ func (s *Server) CheckPrivileges(req PrivilegeCheckRequest) PrivilegeCheckResult // Handle empty username case - but still check root access controls if req.RequestedUsername == "" { - if isPrivilegedOrUnknown(context.currentUser.Username) && !context.allowRoot { + if isPrivilegedOrUnknown(context.currentUser) && !context.allowRoot { return PrivilegeCheckResult{ Allowed: false, Error: &PrivilegedUserError{Username: context.currentUser.Username}, @@ -142,7 +143,7 @@ func (s *Server) checkUserRequest(ctx *privilegeCheckContext, req PrivilegeCheck needsUserSwitching := !isSameResolvedUser(resolvedUser, ctx.currentUser) - if isPrivilegedOrUnknown(resolvedUser.Username) && !ctx.allowRoot { + if isPrivilegedOrUnknown(resolvedUser) && !ctx.allowRoot { return PrivilegeCheckResult{ Allowed: false, Error: &PrivilegedUserError{Username: resolvedUser.Username}, @@ -289,10 +290,11 @@ func isWindowsSameUser(requestedUsername, currentUsername string) bool { return strings.EqualFold(reqDomain, curDomain) } -// isPrivilegedOrUnknown reports whether the given username represents a -// privileged user, or on Windows an account whose privilege could not be -// determined. -// On Unix: root. +// isPrivilegedOrUnknown reports whether the given account is a privileged +// user, or one whose privilege could not be determined. +// On Unix: uid 0, whatever the account is called (FreeBSD ships "toor"), and +// anything named root. An account whose uid is not a number is reported as +// privileged. // 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". An account that cannot be @@ -300,11 +302,14 @@ func isWindowsSameUser(requestedUsername, currentUsername string) bool { // // Use this to refuse privileged accounts, never to grant them anything: the // undetermined case is safe for a refusal and unsafe for a grant. -func isPrivilegedOrUnknown(username string) bool { +func isPrivilegedOrUnknown(account *user.User) bool { if getCurrentOS() != "windows" { - return username == "root" + // Parsed rather than compared with "0": without cgo the uid is the passwd + // field as written, and libc reads "00" as uid 0 too. + uid, err := strconv.ParseUint(account.Uid, 10, 32) + return err != nil || uid == 0 || account.Username == "root" } - return getWindowsAccountPrivilegedOrUnknown(username) + return getWindowsAccountPrivilegedOrUnknown(account.Username) } // isCurrentProcessPrivileged checks if the current process is running with elevated privileges. diff --git a/client/ssh/server/user_utils_test.go b/client/ssh/server/user_utils_test.go index 2fa9b68ee..2d4e13bce 100644 --- a/client/ssh/server/user_utils_test.go +++ b/client/ssh/server/user_utils_test.go @@ -130,6 +130,35 @@ func TestCheckPrivileges_ComprehensiveMatrix(t *testing.T) { expectedAllowed: true, // Should fallback to current user (alice) expectedRequiresSwitch: false, // Fallback means no actual switching }, + // A second uid-0 account is root under another name + { + name: "linux_root_request_uid_0_alias_allowRoot_false", + os: "linux", + euid: 0, + currentUser: createTestUser("root", "0", "0", "/root"), + requestedUsername: "toor", + featureSupportsUserSwitch: true, + allowRoot: false, + lookupUsers: map[string]*user.User{ + "toor": createTestUser("toor", "0", "0", "/root"), + }, + expectedAllowed: false, + expectedRequiresSwitch: false, + }, + { + name: "linux_root_request_uid_0_alias_allowRoot_true", + os: "linux", + euid: 0, + currentUser: createTestUser("root", "0", "0", "/root"), + requestedUsername: "toor", + featureSupportsUserSwitch: true, + allowRoot: true, + lookupUsers: map[string]*user.User{ + "toor": createTestUser("toor", "0", "0", "/root"), + }, + expectedAllowed: true, + expectedRequiresSwitch: false, + }, { name: "windows_admin_can_switch_to_alice", os: "windows", @@ -441,17 +470,22 @@ func TestPrivilegedUsernameDetection(t *testing.T) { tests := []struct { name string username string + uid string platform string privileged bool }{ // Unix/Linux tests - {"unix_root", "root", "linux", true}, - {"unix_regular_user", "alice", "linux", false}, - {"unix_root_capital", "Root", "linux", false}, // Case-sensitive + {"unix_root", "root", "0", "linux", true}, + {"unix_regular_user", "alice", "1000", "linux", false}, + {"unix_root_capital", "Root", "1000", "linux", false}, // Case-sensitive + {"unix_uid_0_under_another_name", "toor", "0", "freebsd", true}, + {"unix_uid_0_with_leading_zero", "toor", "00", "linux", true}, + {"unix_uid_not_a_number", "alice", "+0", "linux", true}, + {"unix_named_root_without_uid_0", "root", "1000", "linux", true}, // Windows dispatch to the (mocked) account classifier - {"windows_administrator", "Administrator", "windows", true}, - {"windows_regular_user", "alice", "windows", false}, + {"windows_administrator", "Administrator", "S-1-5-21-1-500", "windows", true}, + {"windows_regular_user", "alice", "S-1-5-21-1-1001", "windows", false}, } for _, tt := range tests { @@ -460,7 +494,7 @@ func TestPrivilegedUsernameDetection(t *testing.T) { cleanup := setupTestDependencies(nil, nil, tt.platform, 1000, nil, nil) defer cleanup() - result := isPrivilegedOrUnknown(tt.username) + result := isPrivilegedOrUnknown(&user.User{Username: tt.username, Uid: tt.uid}) assert.Equal(t, tt.privileged, result, "privilege classification for %s on %s", tt.username, tt.platform) }) }