[process/windows] - Ignore accessing command line arguments for selec…

…ted processes (currently, lsass.exe) (#198)

See elastic/beats#41407 for more details.

TL;DR;

Processes such as `lsass.exe` has no meaningful cmd line and and can
trigger false positives with Windows ASR rules.
We can find pid for that using `SYSTEM\\CurrentControlSet\\Control\\Lsa`
path

Relates elastic/beats#41407

---------

Co-authored-by: Gabriel Landau <42078554+gabriellandau@users.noreply.github.com>

metric/system/process/helpers_others.go

@@ -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{}{}
+}

metric/system/process/helpers_windows.go

@@ -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 {
@@ -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 {
+		logp.L().Warnw("Failed to open 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{}{}
+	return m
+}

metric/system/process/process.go

@@ -295,15 +295,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, " ")
+		}
 	}
 
 	// network data

metric/system/process/process_common.go

@@ -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
@@ -207,6 +208,7 @@ func (procStats *Stats) Init() error {
 		}
 		procStats.cgroups = cgReader
 	}
+	procStats.excludedPIDs = processesToIgnore()
 	return nil
 }
 

metric/system/process/process_test.go

@@ -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

metric/system/process/process_windows_test.go

@@ -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)
+		}
+	}
+}