From 45f7aa01966f9adb643658b7a6aafa40a5f27f12 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Mon, 27 May 2019 13:45:57 +0200 Subject: [PATCH 1/8] [Metricbeat] Fix system/process* metricsets under Windows This updates vendored elastic/gosigar to v0.10.3, which addresses an issue when listing processes under a non-privileged Windows user. For the system/process metricset, only processes belonging to the user under which Metricbeat is running were reported. For the system/process_summary metricset, process belonging to other users were reported as unknown state. This fix will make Metricbeat able to report all processes, but information about which users owns those processes will be missing. Fixes #12301 --- CHANGELOG.next.asciidoc | 1 + NOTICE.txt | 4 +- .../github.com/elastic/gosigar/CHANGELOG.md | 5 +++ .../elastic/gosigar/sigar_windows.go | 17 ++++---- vendor/vendor.json | 42 +++++++++---------- 5 files changed, 36 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index f24fc4ccb7e..2ee45993775 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -129,6 +129,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix direction of incoming IPv6 sockets. {pull}12248[12248] - Validate that kibana/status metricset cannot be used when xpack is enabled. {pull}12264[12264] - Ignore prometheus metrics when their values are NaN or Inf. {pull}12084[12084] {issue}10849[10849] +- Fix an issue listing all processes when run under Windows as a non-privileged user. {issue}12301[12301] {pull}12305[12305] *Packetbeat* diff --git a/NOTICE.txt b/NOTICE.txt index 38957751f09..b1fff017c55 100644 --- a/NOTICE.txt +++ b/NOTICE.txt @@ -765,8 +765,8 @@ Elasticsearch, B.V. (https://www.elastic.co/). -------------------------------------------------------------------- Dependency: github.com/elastic/gosigar -Version: v0.10.2 -Revision: 1227b9d6877d126ad640087e44439d70dba2df4f +Version: v0.10.3 +Revision: 99ed9cf55303a9d3936cb656b9a86a4a6e67b30a License type (autodetected): Apache-2.0 ./vendor/github.com/elastic/gosigar/LICENSE: -------------------------------------------------------------------- diff --git a/vendor/github.com/elastic/gosigar/CHANGELOG.md b/vendor/github.com/elastic/gosigar/CHANGELOG.md index 04051fed8e8..0ce0fad6f1f 100644 --- a/vendor/github.com/elastic/gosigar/CHANGELOG.md +++ b/vendor/github.com/elastic/gosigar/CHANGELOG.md @@ -12,6 +12,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ### Deprecated +## [0.10.3] + +### Fixed +- ProcState.Get() doesn't fail under Windows when it cannot obtain process ownership information. #121 + ## [0.10.2] ### Fixed diff --git a/vendor/github.com/elastic/gosigar/sigar_windows.go b/vendor/github.com/elastic/gosigar/sigar_windows.go index 66f294e9c14..fc868daf3fc 100644 --- a/vendor/github.com/elastic/gosigar/sigar_windows.go +++ b/vendor/github.com/elastic/gosigar/sigar_windows.go @@ -194,10 +194,11 @@ func (self *ProcState) Get(pid int) error { errs = append(errs, errors.Wrap(err, "getParentPid failed")) } - self.Username, err = getProcCredName(pid) - if err != nil { - errs = append(errs, errors.Wrap(err, "getProcCredName failed")) - } + // getProcCredName will often fail when run as a non-admin user. This is + // caused by strict ACL of the process token belonging to other users. + // Instead of failing completely, ignore this error and still return most + // data with an empty Username. + self.Username, _ = getProcCredName(pid) if len(errs) > 0 { errStrs := make([]string, 0, len(errs)) @@ -274,6 +275,8 @@ func getProcCredName(pid int) (string, error) { if err != nil { return "", errors.Wrapf(err, "OpenProcessToken failed for pid=%v", pid) } + // Close token to prevent handle leaks. + defer token.Close() // Find the token user. tokenUser, err := token.GetTokenUser() @@ -281,12 +284,6 @@ func getProcCredName(pid int) (string, error) { return "", errors.Wrapf(err, "GetTokenInformation failed for pid=%v", pid) } - // Close token to prevent handle leaks. - err = token.Close() - if err != nil { - return "", errors.Wrapf(err, "failed while closing process token handle for pid=%v", pid) - } - // Look up domain account by SID. account, domain, _, err := tokenUser.User.Sid.LookupAccount("") if err != nil { diff --git a/vendor/vendor.json b/vendor/vendor.json index 115959a1af6..75fc8b87546 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -1246,44 +1246,44 @@ "revisionTime": "2018-08-31T13:10:45Z" }, { - "checksumSHA1": "c1rU7WNZ+1AwZcRPBWhPBHcbZjg=", + "checksumSHA1": "tuhGcluN3UtoiFBovqsep6aPx3s=", "path": "github.com/elastic/gosigar", - "revision": "1227b9d6877d126ad640087e44439d70dba2df4f", - "revisionTime": "2019-05-08T13:07:01Z", - "version": "v0.10.2", - "versionExact": "v0.10.2" + "revision": "99ed9cf55303a9d3936cb656b9a86a4a6e67b30a", + "revisionTime": "2019-05-27T11:32:19Z", + "version": "v0.10.3", + "versionExact": "v0.10.3" }, { "checksumSHA1": "TX9y4oPL5YmT4Gb/OU4GIPTdQB4=", "path": "github.com/elastic/gosigar/cgroup", - "revision": "1227b9d6877d126ad640087e44439d70dba2df4f", - "revisionTime": "2019-05-08T13:07:01Z", - "version": "v0.10.2", - "versionExact": "v0.10.2" + "revision": "99ed9cf55303a9d3936cb656b9a86a4a6e67b30a", + "revisionTime": "2019-05-27T11:32:19Z", + "version": "v0.10.3", + "versionExact": "v0.10.3" }, { "checksumSHA1": "hPqGM3DENaGfipEODoyZ4mKogTQ=", "path": "github.com/elastic/gosigar/sys", - "revision": "1227b9d6877d126ad640087e44439d70dba2df4f", - "revisionTime": "2019-05-08T13:07:01Z", - "version": "v0.10.2", - "versionExact": "v0.10.2" + "revision": "99ed9cf55303a9d3936cb656b9a86a4a6e67b30a", + "revisionTime": "2019-05-27T11:32:19Z", + "version": "v0.10.3", + "versionExact": "v0.10.3" }, { "checksumSHA1": "mLq5lOyD0ZU39ysXuf1ETOLJ+f0=", "path": "github.com/elastic/gosigar/sys/linux", - "revision": "1227b9d6877d126ad640087e44439d70dba2df4f", - "revisionTime": "2019-05-08T13:07:01Z", - "version": "v0.10.2", - "versionExact": "v0.10.2" + "revision": "99ed9cf55303a9d3936cb656b9a86a4a6e67b30a", + "revisionTime": "2019-05-27T11:32:19Z", + "version": "v0.10.3", + "versionExact": "v0.10.3" }, { "checksumSHA1": "R70u1XUHH/t1pquvHEFDeUFtkFk=", "path": "github.com/elastic/gosigar/sys/windows", - "revision": "1227b9d6877d126ad640087e44439d70dba2df4f", - "revisionTime": "2019-05-08T13:07:01Z", - "version": "v0.10.2", - "versionExact": "v0.10.2" + "revision": "99ed9cf55303a9d3936cb656b9a86a4a6e67b30a", + "revisionTime": "2019-05-27T11:32:19Z", + "version": "v0.10.3", + "versionExact": "v0.10.3" }, { "checksumSHA1": "Klc34HULvwvY4cGA/D8HmqtXLqw=", From 52891b18cd96764dea00af7bcb93148a9b30593e Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 7 Jun 2019 10:03:21 +0200 Subject: [PATCH 2/8] Added test func in order to provide more information on the failing match --- libbeat/metric/system/process/process.go | 36 ++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index 6cec53a27b4..ca6a453c4b1 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -449,9 +449,9 @@ func (procStats *Stats) GetOne(pid int) (common.MapStr, error) { } newProcs := make(ProcsMap, 1) - p := procStats.getSingleProcess(pid, newProcs) + p, err := procStats.getSingleProcess1(pid, newProcs) if p == nil { - return nil, fmt.Errorf("cannot find matching process for pid=%d", pid) + return nil, fmt.Errorf("GetOne: cannot find matching process for pid=%d with error: %v", pid, err) } e := procStats.getProcessEvent(p) @@ -460,6 +460,38 @@ func (procStats *Stats) GetOne(pid int) (common.MapStr, error) { return e, nil } +func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, error) { + var cmdline string + var env common.MapStr + if previousProc := procStats.ProcsMap[pid]; previousProc != nil { + if procStats.CacheCmdLine { + cmdline = previousProc.CmdLine + } + env = previousProc.Env + } + + process, err := newProcess(pid, cmdline, env) + if err != nil { + return nil, errors.Errorf("Process: Skip process pid=%d: %v", pid, err) + } + + if !procStats.matchProcess(process.Name) { + return nil, errors.Errorf("Process name does not matches the provided regex; pid=%d; name=%s: %v", pid, process.Name, err) + } + + err = process.getDetails(procStats.isWhitelistedEnvVar) + if err != nil { + return nil, errors.Errorf("Error getting process details. pid=%d: %v", process.Pid, err) + logp.Err("Error getting process details. pid=%d: %v", process.Pid, err) + } + + newProcs[process.Pid] = process + last := procStats.ProcsMap[process.Pid] + process.cpuTotalPctNorm, process.cpuTotalPct, process.cpuSinceStart = GetProcCPUPercentage(last, process) + return process, nil +} + + func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { var cmdline string var env common.MapStr From c7a111ded53e121a2c14a45c0be65167d0bda2b7 Mon Sep 17 00:00:00 2001 From: Mariana Date: Fri, 7 Jun 2019 10:18:45 +0200 Subject: [PATCH 3/8] Fix build error --- libbeat/metric/system/process/process.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index ca6a453c4b1..04c3e1481f8 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -469,7 +469,6 @@ func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, } env = previousProc.Env } - process, err := newProcess(pid, cmdline, env) if err != nil { return nil, errors.Errorf("Process: Skip process pid=%d: %v", pid, err) @@ -482,7 +481,6 @@ func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, err = process.getDetails(procStats.isWhitelistedEnvVar) if err != nil { return nil, errors.Errorf("Error getting process details. pid=%d: %v", process.Pid, err) - logp.Err("Error getting process details. pid=%d: %v", process.Pid, err) } newProcs[process.Pid] = process @@ -491,7 +489,6 @@ func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, return process, nil } - func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { var cmdline string var env common.MapStr From 468fc9e60ca3160e818514b458e7b20ecac17eff Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 11 Jun 2019 10:21:27 +0200 Subject: [PATCH 4/8] Removed test func, correcting access rights in sigar_windows file (gosigar pr will follow), test only --- libbeat/metric/system/process/process.go | 33 ++----------------- .../elastic/gosigar/sigar_windows.go | 6 ++-- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index 04c3e1481f8..6cec53a27b4 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -449,9 +449,9 @@ func (procStats *Stats) GetOne(pid int) (common.MapStr, error) { } newProcs := make(ProcsMap, 1) - p, err := procStats.getSingleProcess1(pid, newProcs) + p := procStats.getSingleProcess(pid, newProcs) if p == nil { - return nil, fmt.Errorf("GetOne: cannot find matching process for pid=%d with error: %v", pid, err) + return nil, fmt.Errorf("cannot find matching process for pid=%d", pid) } e := procStats.getProcessEvent(p) @@ -460,35 +460,6 @@ func (procStats *Stats) GetOne(pid int) (common.MapStr, error) { return e, nil } -func (procStats *Stats) getSingleProcess1(pid int, newProcs ProcsMap) (*Process, error) { - var cmdline string - var env common.MapStr - if previousProc := procStats.ProcsMap[pid]; previousProc != nil { - if procStats.CacheCmdLine { - cmdline = previousProc.CmdLine - } - env = previousProc.Env - } - process, err := newProcess(pid, cmdline, env) - if err != nil { - return nil, errors.Errorf("Process: Skip process pid=%d: %v", pid, err) - } - - if !procStats.matchProcess(process.Name) { - return nil, errors.Errorf("Process name does not matches the provided regex; pid=%d; name=%s: %v", pid, process.Name, err) - } - - err = process.getDetails(procStats.isWhitelistedEnvVar) - if err != nil { - return nil, errors.Errorf("Error getting process details. pid=%d: %v", process.Pid, err) - } - - newProcs[process.Pid] = process - last := procStats.ProcsMap[process.Pid] - process.cpuTotalPctNorm, process.cpuTotalPct, process.cpuSinceStart = GetProcCPUPercentage(last, process) - return process, nil -} - func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { var cmdline string var env common.MapStr diff --git a/vendor/github.com/elastic/gosigar/sigar_windows.go b/vendor/github.com/elastic/gosigar/sigar_windows.go index fc868daf3fc..71d31727da6 100644 --- a/vendor/github.com/elastic/gosigar/sigar_windows.go +++ b/vendor/github.com/elastic/gosigar/sigar_windows.go @@ -263,7 +263,7 @@ func getParentPid(pid int) (int, error) { } func getProcCredName(pid int) (string, error) { - handle, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION, false, uint32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) if err != nil { return "", errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } @@ -298,7 +298,7 @@ func getProcCredName(pid int) (string, error) { } func (self *ProcMem) Get(pid int) error { - handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess|windows.PROCESS_VM_READ, false, uint32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) if err != nil { return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } @@ -353,7 +353,7 @@ func (self *ProcArgs) Get(pid int) error { if !version.IsWindowsVistaOrGreater() { return ErrNotImplemented{runtime.GOOS} } - handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess|windows.PROCESS_VM_READ, false, uint32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) if err != nil { return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } From 05fe17df904c288fe4c37f73869761486265779c Mon Sep 17 00:00:00 2001 From: Mariana Date: Tue, 11 Jun 2019 11:48:45 +0200 Subject: [PATCH 5/8] Revert test changes, return debug message for process.getDetails --- libbeat/metric/system/process/process.go | 2 +- vendor/github.com/elastic/gosigar/sigar_windows.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index 6cec53a27b4..9f8ec7aa2bb 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -483,7 +483,7 @@ func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { err = process.getDetails(procStats.isWhitelistedEnvVar) if err != nil { - logp.Err("Error getting process details. pid=%d: %v", process.Pid, err) + logp.Debug("Error getting details for process %s with pid=%s: %v", process.Name, process.Pid, err) return nil } diff --git a/vendor/github.com/elastic/gosigar/sigar_windows.go b/vendor/github.com/elastic/gosigar/sigar_windows.go index 71d31727da6..fc868daf3fc 100644 --- a/vendor/github.com/elastic/gosigar/sigar_windows.go +++ b/vendor/github.com/elastic/gosigar/sigar_windows.go @@ -263,7 +263,7 @@ func getParentPid(pid int) (int, error) { } func getProcCredName(pid int) (string, error) { - handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) + handle, err := syscall.OpenProcess(syscall.PROCESS_QUERY_INFORMATION, false, uint32(pid)) if err != nil { return "", errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } @@ -298,7 +298,7 @@ func getProcCredName(pid int) (string, error) { } func (self *ProcMem) Get(pid int) error { - handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess|windows.PROCESS_VM_READ, false, uint32(pid)) if err != nil { return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } @@ -353,7 +353,7 @@ func (self *ProcArgs) Get(pid int) error { if !version.IsWindowsVistaOrGreater() { return ErrNotImplemented{runtime.GOOS} } - handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess, false, uint32(pid)) + handle, err := syscall.OpenProcess(processQueryLimitedInfoAccess|windows.PROCESS_VM_READ, false, uint32(pid)) if err != nil { return errors.Wrapf(err, "OpenProcess failed for pid=%v", pid) } From 1640681fc7a334c44fcf8eefc0c7fa4fa1fc5ba7 Mon Sep 17 00:00:00 2001 From: Mariana Date: Wed, 12 Jun 2019 09:31:09 +0200 Subject: [PATCH 6/8] Replaced the PR number in the changelog --- CHANGELOG.next.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 2f3e60b3528..5ea1c6dfc61 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -145,7 +145,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Validate that kibana/status metricset cannot be used when xpack is enabled. {pull}12264[12264] - Ignore prometheus metrics when their values are NaN or Inf. {pull}12084[12084] {issue}10849[10849] - In the kibana/stats metricset, only log error (don't also index it) if xpack is enabled. {pull}12265[12265] -- Fix an issue listing all processes when run under Windows as a non-privileged user. {issue}12301[12301] {pull}12305[12305] +- Fix an issue listing all processes when run under Windows as a non-privileged user. {issue}12301[12301] {pull}12475[12475] - Require client_auth by default when ssl is enabled for module http metricset server{pull}12333[12333] *Packetbeat* From fe5f5fb91925e1f75996be7624d75d85fc537ce6 Mon Sep 17 00:00:00 2001 From: Mariana Date: Wed, 12 Jun 2019 10:33:01 +0200 Subject: [PATCH 7/8] Adding selector to log debug message --- libbeat/metric/system/process/process.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index 9f8ec7aa2bb..d2a93f87380 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -483,7 +483,7 @@ func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { err = process.getDetails(procStats.isWhitelistedEnvVar) if err != nil { - logp.Debug("Error getting details for process %s with pid=%s: %v", process.Name, process.Pid, err) + logp.Debug("processes", "Error getting details for process %s with pid=%s: %v", process.Name, process.Pid, err) return nil } From c80604691836d98a2e7778be6fd2b443c0d754d3 Mon Sep 17 00:00:00 2001 From: Mariana Date: Wed, 12 Jun 2019 10:44:54 +0200 Subject: [PATCH 8/8] Wrong type for pid in debug message --- libbeat/metric/system/process/process.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libbeat/metric/system/process/process.go b/libbeat/metric/system/process/process.go index d2a93f87380..544bacc7ec6 100644 --- a/libbeat/metric/system/process/process.go +++ b/libbeat/metric/system/process/process.go @@ -483,7 +483,7 @@ func (procStats *Stats) getSingleProcess(pid int, newProcs ProcsMap) *Process { err = process.getDetails(procStats.isWhitelistedEnvVar) if err != nil { - logp.Debug("processes", "Error getting details for process %s with pid=%s: %v", process.Name, process.Pid, err) + logp.Debug("processes", "Error getting details for process %s with pid=%d: %v", process.Name, process.Pid, err) return nil }