[windows] Windows Defender Data stream overhaul to GA by nicpenning · Pull Request #11249 · elastic/integrations

nicpenning · 2024-09-25T22:11:25Z

Enhancement

Proposed commit message

Overhaul Windows Defender data stream in the Windows integration to make it GA.

Added many ECS fields and removed un-needed fields/processors

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.

Screenshots

nicpenning · 2024-09-25T22:25:30Z

Ready for review and tests.

elasticmachine · 2024-09-25T22:28:32Z

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

elasticmachine · 2024-09-26T07:18:53Z

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

nicpenning · 2024-09-27T02:13:59Z

Made a few corrections. Please review and test now.

intxgo

changes LGTM, but I guess this will fully process only a subset of Windows Defender events. Are the other IDs unimportant or there's just no documentation for them?

packages/windows/data_stream/windows_defender/elasticsearch/ingest_pipeline/default.yml

nicpenning · 2024-10-03T13:23:12Z

changes LGTM, but I guess this will fully process only a subset of Windows Defender events. Are the other IDs unimportant or there's just no documentation for them?

Great question! I targeted all the IDs I observed in my test environment. My hope is that after this beta version of the data stream is out there I can use it at a larger scale and narrow in on more IDs.

The problem is that I can't tell what fields and what type of data is expected in the data from the MS docs.

Also, this hits the primary ones of interest for sure (malware detected/prevented).

It will process most but there will be some casualties, I am sure. Without a full list of Event IDs and their actual field names to expect, it is hard to get this 100% :/

taylor-swanson

Couple of minor comments, otherwise seems good to me

packages/windows/data_stream/windows_defender/elasticsearch/ingest_pipeline/default.yml

…file path extractions.

fearful-symmetry

Logic looks fine, can't comment on the windows-specific fields though.

rdner · 2024-10-14T15:24:33Z

/test

nicpenning · 2024-10-14T18:47:24Z

Looks like pipeline tests failed. I will test locally to see what is going on.

nicpenning · 2024-10-15T01:22:05Z

Doesn't appear that the generated test files were pushed. I will get those pushed as soon as I can.

nicpenning · 2024-10-15T14:26:09Z

Ready for tests again!

rdner · 2024-10-16T17:40:26Z

/test

rdner · 2024-10-16T17:42:05Z

@taylor-swanson could you validate the changes in 3de2f48 please?

elastic-vault-github-plugin-prod · 2024-10-16T18:06:39Z

🚀 Benchmarks report

Package `windows` 👍(5) 💚(3) 💔(1)

Expand to view

Data stream	Previous EPS	New EPS	Diff (%)	Result
`windows_defender`	30303.03	16949.15	-13353.88 (-44.07%)	💔

To see the full report comment with /test benchmark fullreport

elastic-sonarqube · 2024-10-16T18:06:59Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
89.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

elasticmachine · 2024-10-16T18:07:04Z

💚 Build Succeeded

Buildkite Build
Commit: 3de2f48

History

💔 Build #17109 failed 22e796e

taylor-swanson · 2024-10-21T17:13:29Z

@taylor-swanson could you validate the changes in 3de2f48 please?

I think the test results are good.

At first I thought an event was removed, but it was the git diff that tripped me up. It looks like one of the test cases (event ID 1116) was moved to the top and that can cause some misleading git diffs as a result. This includes extra lines showing up in the diff even though the results didn't really change. This is why I try to avoid reordering test cases, since the resulting git diffs tend to be misleading. Everything seems to check out, but feel free to point out something that's suspicious if I missed something.
It looks like the user_data field was removed and was replaced with an enriched event_data field. The PR doesn't mention why this is the case, but I'll assume we have better sample data now and are including it in the tests.
The rest of the changes to the expected test file are either from fields being sorted or updates from the changes in the pipeline.

elastic-vault-github-plugin-prod · 2024-10-21T17:58:36Z

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

nicpenning · 2024-10-22T04:06:54Z

@taylor-swanson could you validate the changes in 3de2f48 please?

I think the test results are good.

At first I thought an event was removed, but it was the git diff that tripped me up. It looks like one of the test cases (event ID 1116) was moved to the top and that can cause some misleading git diffs as a result. This includes extra lines showing up in the diff even though the results didn't really change. This is why I try to avoid reordering test cases, since the resulting git diffs tend to be misleading. Everything seems to check out, but feel free to point out something that's suspicious if I missed something.

It looks like the user_data field was removed and was replaced with an enriched event_data field. The PR doesn't mention why this is the case, but I'll assume we have better sample data now and are including it in the tests.

The rest of the changes to the expected test file are either from fields being sorted or updates from the changes in the pipeline.

Good observations.

I did reorder because I wanted the system test to use this more practical event in the docs. Also when I generated improved json that maps closer to the real world, I used it as is and didn't try to put them back in the right order.

As for the user fields, those were likely residual fields from other windows integrations that this one didn't really use so I capitalized on what actual event fields are being used in the event IDs I was able to generate for my tests.

I will be cautious next time keeping these event IDs in the proper order. Thanks for the review!

* Defender Data stream overhaul to GA * Adjust pipeline to ensure event type is applied * Update Readme * Improve test data with event_data blocks, switch to GSUB and SET for file path extractions. * Generated new JSON test files

nicpenning added 2 commits September 25, 2024 17:07

Defender Data stream overhaul to GA

91cc0a0

Update change log

1a0b639

andrewkroh added bugfix Pull request that fixes a bug issue Integration:windows Windows labels Sep 25, 2024

nicpenning added 2 commits September 25, 2024 17:23

Merge branch 'main' into defender_ecs_map_and_ga

ec61cb3

Update changelog, again.

7c21eeb

nicpenning marked this pull request as ready for review September 25, 2024 22:25

nicpenning requested review from a team as code owners September 25, 2024 22:25

nicpenning requested review from VihasMakwana and rdner September 25, 2024 22:25

andrewkroh added the Team:Security-Windows Platform Security Windows Platform team [elastic/sec-windows-platform] label Sep 25, 2024

pierrehilbert added the Team:Elastic-Agent-Data-Plane Agent Data Plane team [elastic/elastic-agent-data-plane] label Sep 26, 2024

nicpenning added 3 commits September 26, 2024 08:22

Fix changelog YML

f38677c

Adjust pipeline to ensure event type is applied

f84a9ce

Readme updated

7a3cc68

intxgo reviewed Oct 3, 2024

View reviewed changes

packages/windows/data_stream/windows_defender/elasticsearch/ingest_pipeline/default.yml Show resolved Hide resolved

packages/windows/data_stream/windows_defender/elasticsearch/ingest_pipeline/default.yml Show resolved Hide resolved

intxgo approved these changes Oct 4, 2024

View reviewed changes

taylor-swanson reviewed Oct 7, 2024

View reviewed changes

Improve test data with event_data blocks, switch to GSUB and SET for …

22e796e

…file path extractions.

fearful-symmetry approved these changes Oct 9, 2024

View reviewed changes

Generated new JSON test files

3de2f48

rdner requested a review from taylor-swanson October 16, 2024 17:42

rdner merged commit baf51ab into elastic:main Oct 21, 2024

nicpenning deleted the defender_ecs_map_and_ga branch October 22, 2024 04:03

Conversation

nicpenning commented Sep 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Screenshots

Uh oh!

nicpenning commented Sep 25, 2024

Uh oh!

elasticmachine commented Sep 25, 2024

Uh oh!

elasticmachine commented Sep 26, 2024

Uh oh!

nicpenning commented Sep 27, 2024

Uh oh!

intxgo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nicpenning commented Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

taylor-swanson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fearful-symmetry left a comment

Choose a reason for hiding this comment

Uh oh!

rdner commented Oct 14, 2024

Uh oh!

nicpenning commented Oct 14, 2024

Uh oh!

nicpenning commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicpenning commented Oct 15, 2024

Uh oh!

rdner commented Oct 16, 2024

Uh oh!

rdner commented Oct 16, 2024

Uh oh!

elastic-vault-github-plugin-prod bot commented Oct 16, 2024

🚀 Benchmarks report

Package windows 👍(5) 💚(3) 💔(1)

Uh oh!

elastic-sonarqube bot commented Oct 16, 2024

Quality Gate passed

Uh oh!

elasticmachine commented Oct 16, 2024

💚 Build Succeeded

History

Uh oh!

taylor-swanson commented Oct 21, 2024

Uh oh!

elastic-vault-github-plugin-prod bot commented Oct 21, 2024

Uh oh!

nicpenning commented Oct 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nicpenning commented Sep 25, 2024 •

edited

Loading

nicpenning commented Oct 3, 2024 •

edited

Loading

nicpenning commented Oct 15, 2024 •

edited

Loading

Package `windows` 👍(5) 💚(3) 💔(1)