Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[process/windows] - Ignore accessing command line arguments for selected processes (currently, lsass.exe) #198

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions metric/system/process/helpers_others.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,7 @@ func isNonFatal(err error) bool {
errors.Is(err, syscall.EINVAL) ||
errors.Is(err, NonFatalErr{}))
}

func processesToIgnore() map[uint64]struct{} {
return map[uint64]struct{}{}
}
24 changes: 24 additions & 0 deletions metric/system/process/helpers_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (
"syscall"

"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"

"github.com/elastic/elastic-agent-libs/logp"
)

func isNonFatal(err error) bool {
Expand All @@ -35,3 +38,24 @@ func isNonFatal(err error) bool {
errors.Is(err, syscall.EINVAL) ||
errors.Is(err, windows.ERROR_INVALID_PARAMETER) || errors.Is(err, NonFatalErr{})
}

func processesToIgnore() map[uint64]struct{} {
m := make(map[uint64]struct{})
// processesToIgnore checks if we should ignore the pid, to avoid elevated permissions

// LSASS.exe is a process which has no useful cmdline arguments, we should ignore acessing such process to avoid triggering Windows ASR rules
// we can query pid for LASASS.exe from registry
key, err := registry.OpenKey(registry.LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Control\\Lsa", registry.READ)
if err != nil {
swiatekm marked this conversation as resolved.
Show resolved Hide resolved
logp.L().Warnw("Failed to read registry path SYSTEM\\CurrentControlSet\\Control\\Lsa", "error", err)
return m
}
defer key.Close()
lsassPid, _, err := key.GetIntegerValue("LsaPid")
if err != nil {
logp.L().Warnw("Failed to read pid for lsass.exe", "error", err)
return m
}
m[lsassPid] = struct{}{}
VihasMakwana marked this conversation as resolved.
Show resolved Hide resolved
return m
}
18 changes: 10 additions & 8 deletions metric/system/process/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,17 @@ func (procStats *Stats) pidFill(pid int, filter bool) (ProcState, bool, error) {

} // end cgroups processor

status, err = FillMetricsRequiringMoreAccess(pid, status)
if err != nil {
procStats.logger.Debugf("error calling FillMetricsRequiringMoreAccess for pid %d: %w", pid, err)
}
if _, isExcluded := procStats.excludedPIDs[uint64(pid)]; !isExcluded {
status, err = FillMetricsRequiringMoreAccess(pid, status)
if err != nil {
procStats.logger.Debugf("error calling FillMetricsRequiringMoreAccess for pid %d: %w", pid, err)
}

// Generate `status.Cmdline` here for compatibility because on Windows
// `status.Args` is set by `FillMetricsRequiringMoreAccess`.
if len(status.Args) > 0 && status.Cmdline == "" {
status.Cmdline = strings.Join(status.Args, " ")
// Generate `status.Cmdline` here for compatibility because on Windows
// `status.Args` is set by `FillMetricsRequiringMoreAccess`.
if len(status.Args) > 0 && status.Cmdline == "" {
status.Cmdline = strings.Join(status.Args, " ")
}
VihasMakwana marked this conversation as resolved.
Show resolved Hide resolved
}

// network data
Expand Down
2 changes: 2 additions & 0 deletions metric/system/process/process_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ type Stats struct {
cgroups *cgroup.Reader
logger *logp.Logger
host types.Host
excludedPIDs map[uint64]struct{} // List of PIDs to ignore while calling FillMetricsRequiringMoreAccess
}

// PidState are the constants for various PID states
Expand Down Expand Up @@ -207,6 +208,7 @@ func (procStats *Stats) Init() error {
}
procStats.cgroups = cgReader
}
procStats.excludedPIDs = processesToIgnore()
return nil
}

Expand Down
26 changes: 26 additions & 0 deletions metric/system/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,32 @@ func TestIncludeTopProcesses(t *testing.T) {
}
}

func TestProcessesExcluded(t *testing.T) {
state := Stats{
Procs: []string{".*"},
Hostfs: resolve.NewTestResolver("/"),
CPUTicks: false,
}
require.NoError(t, state.Init())

_, processes, err := state.Get()
assert.True(t, isNonFatal(err), fmt.Sprintf("Fatal Error: %s", err))

for _, pMap := range processes {
iPid, err := pMap.GetValue("process.pid")
require.NoError(t, err)
pid, ok := iPid.(int) // to avoid panics
require.True(t, ok)
if _, excluded := state.excludedPIDs[uint64(pid)]; excluded {
// if pid is excluded, its arglist amd commandline should be empty
ok, _ = pMap.HasKey("process.args")
require.False(t, ok)
ok, _ = pMap.HasKey("process.command_line")
require.False(t, ok)
}
}
}

// runThreads run the threads binary for the current GOOS.
//
//go:generate docker run --rm -v ./testdata:/app --entrypoint g++ docker.elastic.co/beats-dev/golang-crossbuild:1.21.0-main -pthread -std=c++11 -o /app/threads /app/threads.cpp
Expand Down
13 changes: 13 additions & 0 deletions metric/system/process/process_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,16 @@ func TestGetInfoForPid_numThreads(t *testing.T) {
numThreads, want, expected)
}
}

func TestLsassFound(t *testing.T) {
processNames := []string{"lsass.exe"}
m := processesToIgnore()
require.NotEmpty(t, m)

for pid := range m {
state, err := GetInfoForPid(resolve.NewTestResolver("/"), int(pid))
if err == nil {
require.Contains(t, processNames, state.Name)
}
}
}
Loading