Skip to content

Commit

Permalink
Fix admin permission check on localized windows (#1552) (#1554)
Browse files Browse the repository at this point in the history
Fix admin permission check on localized windows (#1552)

(cherry picked from commit 33a5f7e)

Co-authored-by: Michal Pristas <michal.pristas@gmail.com>
  • Loading branch information
mergify[bot] and michalpristas authored Oct 18, 2022
1 parent 6443f11 commit 36ffd9a
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: bug-fix

# Change summary; a 80ish characters long description of the change.
summary: Fix admin permission check on localized windows

# Long description; in case the summary is not enough to describe the change
# this field accommodate a description without length limits.
#description:

# Affected component; a word indicating the component this changeset affects.
component:

# PR number; optional; the PR number that added the changeset.
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
# Please provide it if you are adding a fragment for a different PR.
pr: 1552

# Issue number; optional; the GitHub issue related to this changeset (either closes or is part of).
# If not present is automatically filled by the tooling with the issue linked to the PR number.
issue: 857
47 changes: 42 additions & 5 deletions internal/pkg/agent/control/server/listener_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ package server
import (
"net"
"os/user"
"strings"

"github.com/pkg/errors"

Expand All @@ -18,9 +19,14 @@ import (
"github.com/elastic/elastic-agent/pkg/core/logger"
)

const (
NTAUTHORITY_SYSTEM = "S-1-5-18"
ADMINISTRATORS_GROUP = "S-1-5-32-544"
)

// createListener creates a named pipe listener on Windows
func createListener(_ *logger.Logger) (net.Listener, error) {
sd, err := securityDescriptor()
func createListener(log *logger.Logger) (net.Listener, error) {
sd, err := securityDescriptor(log)
if err != nil {
return nil, err
}
Expand All @@ -31,7 +37,7 @@ func cleanupListener(_ *logger.Logger) {
// nothing to do on windows
}

func securityDescriptor() (string, error) {
func securityDescriptor(log *logger.Logger) (string, error) {
u, err := user.Current()
if err != nil {
return "", errors.Wrap(err, "failed to get current user")
Expand All @@ -42,11 +48,42 @@ func securityDescriptor() (string, error) {
// 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 u.Username == "NT AUTHORITY\\SYSTEM" {

if isAdmin, err := isWindowsAdmin(u); 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 {
// running as SYSTEM, include Administrators group so Administrators can talk over
// the named pipe to the running Elastic Agent system process
// https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems
descriptor += "(A;;GA;;;S-1-5-32-544)" // Administrators group
descriptor += "(A;;GA;;;" + ADMINISTRATORS_GROUP + ")"
}
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, errors.Wrap(err, "failed to get current user groups")
}

for _, groupSid := range groups {
if equalsSystemGroup(groupSid) {
return true, nil
}
}

return false, nil
}

func equalsSystemGroup(s string) bool {
return strings.EqualFold(s, NTAUTHORITY_SYSTEM) || strings.EqualFold(s, ADMINISTRATORS_GROUP)
}

0 comments on commit 36ffd9a

Please sign in to comment.