-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Winlogbeat] Add Powershell logging module #18526
[Winlogbeat] Add Powershell logging module #18526
Conversation
x-pack/winlogbeat/module/powershell/test/testdata/800.evtx.golden.json
Outdated
Show resolved
Hide resolved
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
Pinging @elastic/siem (Team:SIEM) |
872eb74
to
4eb33ee
Compare
f46c53b
to
84cbf38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exciting! I want to get this deployed after it merges and test it out some more.
to: "process.command_line", | ||
}, | ||
{ | ||
from: "winlog.event_data.HostName", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HostName field is included in message details of events identified by EID 400 and EID 403. For a local activity, this filed is recorded as ConsoleHost (HostName = ConsoleHost); for a remote activity handled by PowerShell, HostName is recorded as ServerRemoteHost (HostName = ServerRemoteHost) on the system that is accessed. https://nsfocusglobal.com/Attack-and-Defense-Around-PowerShell-Event-Logging
If the values truly are ConsoleHost
and ServerRemoteHost
then maybe we can find a different place for this than process.title
. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I thought those two were the only possible values, but others seem to also be possible e.g.: Windows PowerShell ISE Host
in the test evtx
files. So I would expect it to be anything else at some point, with those two being possibly the most common ones when executing through local or remote shell. I agree that maybe process.title
is not the right place, maybe something like powershell.process.name
following with powershell.process.executable_version
that is already there? wdyt?
A few ideas on the dashboard... In general think about what signals are useful to the user. The dashboards can have a few different audiences. So in an overview like this I like provide some basic metrics that show Winlogbeat is working and collecting data. Then provide some more detailed visualizations to help understand the PowerShell activity across all the hosts. With the visualizations try to make it possible for the user to "drill down" into the data. Maybe add some high-level metrics across the top (total commands, total remote commands, unique users, unique hosts, unique powershell versions). I would like to have a host component to the dashboard. Like incorporate some host counts and maybe a top-N hosts table for powershell command activity. This way the dashboard works well when you have more than a single host you're looking at. Could the engine and command starts could be combined line chart over time (with two series being charted)? Are the stops events useful visually, if not I'd omit them. It might also be useful to add a saved search to the bottom showing the actual process arguments. This way as you filter by clicking on the visualizations you can see the associated raw data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good.
Couple of suggestion on ECS categorization:
For the current IDs, could all of them have event.category = process ?
For 4105 event.type could probably be start
For 4106 event.type could probably be stop
For the others event.type could probably be info
could winlog.record_id be mapped to event.id? I'm not sure if it is unique or not.
Optional style suggestion.
It looks like Microsoft sometimes uses spaces in the key names and sometimes not. I'm thinking it might be helpful to go through all the keys and normalize them to one or the other (or snake case). It would make it easier in the future and you could convert a lot of your calls to popFirstFoundFromEventData to a simple rename processor.
BTW Nice job at figuring out all of the dashboard exporting logistics. You'll want to include a screenshot of the final dashboard in the module docs. |
They are only unique within an event log on a host IIRC. Does that qualify? A unique ID could be created by fingerprinting |
81af99e
to
7401f6f
Compare
6f3634f
to
0b7b2c3
Compare
For future PRs, it's favorable from a reviewer standpoint if you squash at the end. This way we can review the changes since we last reviewed and save time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Though looks like mage update
needs run to get the build green. We might want to shrink down the screenshot dimensions before merging. Let's see what @dedemorton thinks?
@@ -2,11 +2,13 @@ | |||
This file is generated! See scripts/mage/docs.go or run 'mage docs'. | |||
//// | |||
|
|||
* <<{beatname_lc}-module-powershell,Powershell>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* <<{beatname_lc}-module-powershell,Powershell>> | |
* <<{beatname_lc}-module-powershell,PowerShell>> |
Yes, I would make it smaller. The general guideline is to "take screenshots with an aspect ratio of 16:9 (1920x1080 preferred, also commonly referred to as 1080p)." Though honestly, this is the marketing guideline; most of screenshots in our docs do not currently follow that guideline. Maybe just resize to 1920 width (preserving aspect ratio) and see how it looks in the built docs. For internal folks: you'll find screen capture guidelines under "Guidelines – Screenshots" in the company wiki. |
5786f13
to
7fd374d
Compare
Initial support for event ids: 400, 403, 600, 800, 4103, 4014, 4105, 4106 Add fields documentation Add powershell module dashboard Closes elastic#16262
7fd374d
to
873dbd0
Compare
@dedemorton I resized the images to be 1920w, let me know if they need anything else 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Initial support for event ids: 400, 403, 600, 800, 4103, 4014, 4105, 4106 Add fields documentation Add powershell module dashboard Closes elastic#16262 (cherry picked from commit f4019d5)
Nice work! Is there any way to add a flag to not delete the source fields? I like all the extractions but we also need to keep source data in tact. One example, reassembling the details is very helpful for detection but can sometimes cause issues for compliance that need to recreate the source data. |
@gwsales Sounds like a good idea. Can you please open a new issue for this? |
What does this PR do?
Adds PowerShell logging module to winlogbeat.
Why is it important?
Add a new Winlogbeat module to collect logs from PowerShell. This will collect information about the scripts and modules that are being executed.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Module Checklist
Handle events
Related issues
Closes #16262