From c02f65a3e5d2ca8b18589a544b710fb9949c055c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 4 Jan 2024 14:13:00 -0600 Subject: [PATCH] switch to consistent HasRoot method on windows and fix memory leak (#3988) (#4004) (cherry picked from commit 9500980bcca65053a2c909e7210d3a63281275c6) Co-authored-by: Lee E Hinman <57081003+leehinman@users.noreply.github.com> --- pkg/control/v2/server/listener_windows.go | 37 ++--------------------- pkg/utils/root_windows.go | 10 ++++-- 2 files changed, 10 insertions(+), 37 deletions(-) diff --git a/pkg/control/v2/server/listener_windows.go b/pkg/control/v2/server/listener_windows.go index 6ce0898192a..55e0ecf3cda 100644 --- a/pkg/control/v2/server/listener_windows.go +++ b/pkg/control/v2/server/listener_windows.go @@ -10,7 +10,6 @@ import ( "fmt" "net" "os/user" - "strings" "github.com/elastic/elastic-agent-libs/api/npipe" @@ -37,14 +36,10 @@ func securityDescriptor(log *logger.Logger) (string, error) { if err != nil { return "", fmt.Errorf("failed to get current user: %w", err) } - // Named pipe security and access rights. - // We create the pipe and the specific users should only be able to write to it. - // See docs: https://docs.microsoft.com/en-us/windows/win32/ipc/named-pipe-security-and-access-rights - // String definition: https://docs.microsoft.com/en-us/windows/win32/secauthz/ace-strings - // Give generic read/write access to the specified user. + descriptor := "D:P(A;;GA;;;" + u.Uid + ")" - if isAdmin, err := isWindowsAdmin(u); err != nil { + if isAdmin, err := utils.HasRoot(); err != nil { // do not fail, agent would end up in a loop, continue with limited permissions log.Warnf("failed to detect admin: %w", err) } else if isAdmin { @@ -53,32 +48,6 @@ func securityDescriptor(log *logger.Logger) (string, error) { // https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems descriptor += "(A;;GA;;;" + utils.AdministratorSID + ")" } - return descriptor, nil -} - -func isWindowsAdmin(u *user.User) (bool, error) { - if u.Username == "NT AUTHORITY\\SYSTEM" { - return true, nil - } - - if equalsSystemGroup(u.Uid) || equalsSystemGroup(u.Gid) { - return true, nil - } - groups, err := u.GroupIds() - if err != nil { - return false, fmt.Errorf("failed to get current user groups: %w", err) - } - - for _, groupSid := range groups { - if equalsSystemGroup(groupSid) { - return true, nil - } - } - - return false, nil -} - -func equalsSystemGroup(s string) bool { - return strings.EqualFold(s, utils.SystemSID) || strings.EqualFold(s, utils.AdministratorSID) + return descriptor, nil } diff --git a/pkg/utils/root_windows.go b/pkg/utils/root_windows.go index 0a24fc3757f..3881f52fcdc 100644 --- a/pkg/utils/root_windows.go +++ b/pkg/utils/root_windows.go @@ -7,7 +7,8 @@ package utils import ( - "github.com/pkg/errors" + "fmt" + "golang.org/x/sys/windows" ) @@ -28,14 +29,17 @@ func HasRoot() (bool, error) { 0, 0, 0, 0, 0, 0, &sid) if err != nil { - return false, errors.Errorf("sid error: %s", err) + return false, fmt.Errorf("allocate sid error: %w", err) } + defer func() { + _ = windows.FreeSid(sid) + }() token := windows.Token(0) member, err := token.IsMember(sid) if err != nil { - return false, errors.Errorf("token membership error: %s", err) + return false, fmt.Errorf("token membership error: %w", err) } return member, nil