Apply the SSH root-login setting to uid 0, not only to the account named root (#8390)

This commit is contained in:
Viktor Liu
2026-10-29 00:21:01 +09:00
committed by GitHub
parent 1acdebf395
commit e2ee1dd002
3 changed files with 55 additions and 16 deletions
+1 -1
View File
@@ -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)
})
}
+14 -9
View File
@@ -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.
+40 -6
View File
@@ -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)
})
}