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

MetricBeat is Triggering Avoidable Credential Dumping False Positives #41407

Open
gabriellandau opened this issue Oct 23, 2024 · 8 comments
Open
Assignees
Labels
bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Comments

@gabriellandau
Copy link

gabriellandau commented Oct 23, 2024

MetricBeat uses gosigar for ProcMem and ProcArgs. Both of these calls request PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ which can trigger credential dumping false positives in third-party security software. We can avoid this problem with two simple changes, reducing SDH, customer headache, and vulnerabilities/blindspots created through customer exceptions in their security software. As a benefit, we'll be able to collect some data that we couldn't collect previously.

ProcMem::Get()

ProcMem::Get() requests PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ here. The handle is for GetProcessMemoryInfo. The docs for GetProcessMemoryInfo state that PROCESS_VM_READ is only needed for XP and Server 2003. Agent no longer supports either of those platforms, so let's stop requesting/requiring PROCESS_VM_READ.

Another important point is that we will never successfully acquire a PROCESS_VM_READ handle on any Protected Process or PPL, meaning this function will always fail on those processes. Since it's unnecessary for GetProcessMemoryInfo, then this function is failing needlessly on such processes. As a simple test for this, check whether you are getting memory information for services.exe which always runs as PPL.

ProcArgs::Get()

ProcArgs::Get() requests PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ here. lsass.exe never has a meaningful command line (screenshot below), so can we skip calling ProcArgs::Get() on lsass.exe?

Its PID can be found in the registry. We can query that value once and cache the value because it will never change until reboot. This LSA exclusion logic may be better put outside of gosigar itself.

C:\>reg query HKLM\SYSTEM\CurrentControlSet\Control\Lsa /v LsaPid

HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Lsa
    LsaPid    REG_DWORD    0x5ac

Image

For confirmed bugs, please report:

  • Version: 8.15.0
  • Operating System: Windows
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 23, 2024
@gabriellandau gabriellandau changed the title MetricBeat is Triggering Credential Dumping False Positives MetricBeat is Triggering Avoidable Credential Dumping False Positives Oct 23, 2024
@leehinman leehinman added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Oct 23, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 23, 2024
@gabriellandau
Copy link
Author

@cmacknz
Copy link
Member

cmacknz commented Oct 23, 2024

FYI @VihasMakwana since you have been addressing some of the other unnecessary sources of errors in the system module.

@VihasMakwana
Copy link
Contributor

VihasMakwana commented Dec 5, 2024

@gabriellandau
For process memory:
- metricbeat uses elastic-agent-system-metrics's custom implementation, not gosigar. See here.
- It only uses PROCESS_QUERY_LIMITED_INFORMATION and not PROCESS_VM_READ(thanks to this PR).

For process args:
- Again, metricbeat uses elastic-agent-system-metrics's custom implementation, not gosigar. See here.
- Additionally, due to recent changes, API calls that require PROCESS_VM_READ are no longer guaranteed to succeed (especially for protected processes, as you mentioned). As a result, we log any errors at the debug level and avoid propagating them to the caller.

@VihasMakwana
Copy link
Contributor

We should probably update gosigar for handling the process memory without PROCESS_VM_READ.

But as far as metricbeat is concerned, I don't think we need to make any additional changes.

cc: @cmacknz

@cmacknz
Copy link
Member

cmacknz commented Dec 5, 2024

We should probably update gosigar for handling the process memory without PROCESS_VM_READ.

Makes sense to me. We still depend on gosigar so fixing it there will potentially save us from accidentally regressing on this somewhere else in the future.

@gabriellandau
Copy link
Author

For process args:
- Again, metricbeat uses elastic-agent-system-metrics's custom implementation, not gosigar. See here.
- Additionally, due to recent changes, API calls that require PROCESS_VM_READ are no longer guaranteed to succeed (especially for protected processes, as you mentioned). As a result, we log any errors at the debug level and avoid propagating them to the caller.

Thanks for clarifying. So the original question remains:

lsass.exe never has a meaningful command line (screenshot below), so can we skip calling ProcArgs::Get() on lsass.exe?

@VihasMakwana
Copy link
Contributor

elastic/elastic-agent-system-metrics#198 up for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

No branches or pull requests

5 participants