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

[windows] Fix parsing of event data fields for event 600 #9490

Merged
merged 4 commits into from Apr 5, 2024

Conversation

marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Apr 2, 2024

Proposed commit message

Some 600 powershell events can contain multiline values, meaning the current KV split is not enough to handle them. This adds specific logic to handle these.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Closes #9469

@marc-gr marc-gr marked this pull request as ready for review April 2, 2024 13:18
@marc-gr marc-gr requested review from a team as code owners April 2, 2024 13:18
@elasticmachine
Copy link

Pinging @elastic/sec-windows-platform (Team:Security-Windows Platform)

@andrewkroh
Copy link
Member

fyi I was checking our "SEI Demo" cluster and I see that error for 600, 403, and 400.

Screenshot 2024-04-02 at 10 24 13

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some of the args contain "\n", not sure if that is expected or not.

@marc-gr marc-gr requested a review from leehinman April 3, 2024 06:27
@elasticmachine
Copy link

elasticmachine commented Apr 3, 2024

🚀 Benchmarks report

Package windows 👍(4) 💚(2) 💔(2)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
applocker_msi_and_script 9523.81 7352.94 -2170.87 (-22.79%) 💔
powershell 2392.34 1506.02 -886.32 (-37.05%) 💔

To see the full report comment with /test benchmark fullreport

@marc-gr marc-gr enabled auto-merge (squash) April 3, 2024 12:34
Split Events 4xx and 600 event data fields.
Some events can contain multiline values containing also '\n', '\s', and '=' characters,
for this reason a simple KV processor is not reliable enough and we need a more specific parsing.
lang: painless
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source: |-
def p = ctx.winlog?.event_data[params["field"]];
// Define the pattern that will match all keys
def pat = /(^|(^[\n]?))?\t([^\s\W]+)=/m;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this regex assumes that the powershell script will be pretty print with indentation.

The loop will skip over such line form the event example
\n\t\t\t\t\t\t\t[Parameter(ValueFromPipeline=\"\", ValueFromPipelineByPropertyName=\"\",
But if the script was not print with identation, could this became
\n\t[Parameter(ValueFromPipeline=\"\", ValueFromPipelineByPropertyName=\"\",
and break the parser?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that the idea was to put the whole function body under one key
[HostApplication] = [C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe function...]

Copy link

@intxgo intxgo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the KV parsing might be misaligned in some corner cases, but it'll always parse the content without error, which is an improvement.

@marc-gr marc-gr merged commit 803c844 into elastic:main Apr 5, 2024
3 checks passed
@elasticmachine
Copy link

💚 Build Succeeded

History

Copy link

@elasticmachine
Copy link

Package windows - 1.44.5 containing this change is available at https://epr.elastic.co/search?package=windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[windows.powershell] Pipeline error while handling event ID 600
5 participants