[M365 Defender] Improve ECS mapping and fix incorrect or missing fields by P1llus · Pull Request #7522 · elastic/integrations

P1llus · 2023-08-24T06:59:09Z

What does this PR do?

This rewamps the Event datastream, and does some bigger format changes, which reflects in the git diff.
When reviewing this, since many existing processors and definitions have simply moved to another location, its better to review these ingest pipelines as if they were new, rather than looking at what changed.

This is a list of changed applied:

General Changes:

Fixed up event.kind/error.message and tagging for processors.
Grouped processors together in pipelines based on what fields and logic is applied, this was necessary specially on the device pipeline.
Added comments to explain mapping logic when applicable (mostly device pipeline).
Changed set processors that adds array values for fields like event.category to append processors instead, so values are not overwritten.
Removed unecessary convert processors when fields are already in the correct type.

Alert Pipeline Changes:

Changed threat.enrichment.indicator to threat.indicator, as the enrichment fields are meant to be used by Indicator SIEM rules.
Added missing custom fields in processors and fields.yml
Mapping logic need to consider if an alert is an indicator or not (when a field should be mapped to threat.*)

Device Pipeline Changes:

A large amount of field mappings are incorrect due to MS re-using fields for multiple usecases (RemoteIP might both be a source or destination IP for example. Or different scenarios requires Processor mappings to go either to primary or parent). This logic has been updated.
Removed appending to source/destination.ip, as the multiple fields that maps to the same ECS fields should not be available at the same time.
Added logic for network direction on certain processors, to map them correctly.
Added support for more device events.
Added support for DLL ECS fields.

App/Email/Identity Pipeline Changes:

Changes to these pipelines are mostly cosmetic, so that all pipelines follow the same grouping format, making them easier to read, review and correct minor mistakes that existed in the current pipelines.

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

Event datastream dashboards was rewritten, while Incidents (Incidents and Alerts) simply had their table of contents added.
Event datastream:

Incidents datastream:

elasticmachine · 2023-08-24T07:20:25Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-09-05T10:24:27.185+0000
Duration: 16 min 23 sec

Test stats 🧪

Test	Results
Failed	0
Passed	26
Skipped	0
Total	26

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2023-08-24T07:20:36Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`3/3`)	💚
Files	100.0% (`7/7`)	💚 3.358
Classes	100.0% (`7/7`)	💚 3.358
Methods	85.556% (`77/90`)	👎 -6.768
Lines	92.596% (`5590/6037`)	👍 4.325
Conditionals	100.0% (`0/0`)	💚

… making the pipeline much easier to navigate and review

…was unecessary

…thers

… errors

efd6

I haven't taken a look at the dashboards.

efd6 · 2023-08-31T00:42:00Z

+      copy_from: m365_defender.event.account.sid
+      ignore_empty_value: true
+      tag: set_user_id
+      if: ctx.m365_defender?.event?.category?.toLowerCase().contains('identitylogonevents')


This is unsafe; in the case of a null at any stage in the accessor path, we end up calling a method on null. So we need to have a separate null check and short circuit.

While this field can never be null, we can at least check if ctx.m365_defender.event.category is null first, if there comes a time that event is null, then the document is empty, and something more serious has happened.

Will add a commit to add nullchecks for category

ctx.m365_defender?.event?.category is a required field in default.yml to run the other pipelines, and can never be empty.
The field is also set by Event Hub, so if this field is missing, no parsing would be done either way. Looking at the massive amount of null-checks needed for ctx.m365_defender?.event?.category, do we really want to keep them?

Can we have a drop processor at the top with a comment explaining this for the case when ctx.m365_defender?.event?.category == null. That would obviate all the checks.

I think a drop is a bit much, but I can add an error.message maybe, saying that the field is missing, just in case.

For the invariant to be usable it must be a drop. If it can't be a drop, then we don't have the invariant and all the null checks must be in place.

P1llus · 2023-09-04T09:56:37Z

Added screenshots, dashboards and fixes from PR comments

elasticmachine · 2023-09-04T09:57:35Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

efd6

LGTM, but please wait for @kcreddy

kcreddy

LGTM 👍🏼

…ds (elastic#7522) * draft commit * adding testdata * fixing category if condition logic to use contains * Moves all the processors around and groups them by logic and usecase, making the pipeline much easier to navigate and review * Add rename processors for previously removed convert processors that was unecessary * group processors in alert pipeline together, similar to device pipeline * group app_and_identify processors together to fit the format of the others * updating ecs.yml and fields.yml definitions * Update conditions for number to boolean conditions to prevent casting errors * stash dashboard changes * adding all dashboards and new screenshots * added fixes to pipelines from PR comments * added major version bump * adding PR comments and updating generated test files

draft commit

5bb8e5e

P1llus added enhancement New feature or request Team:Security-External Integrations Integration:m365_defender Microsoft Defender XDR labels Aug 24, 2023

P1llus added 9 commits August 24, 2023 10:05

adding testdata

7604f8a

fixing category if condition logic to use contains

e01b50d

Moves all the processors around and groups them by logic and usecase,…

f9b8f35

… making the pipeline much easier to navigate and review

Add rename processors for previously removed convert processors that …

7a38dd5

…was unecessary

group processors in alert pipeline together, similar to device pipeline

bf3dd3f

group app_and_identify processors together to fit the format of the o…

cc55bd0

…thers

updating ecs.yml and fields.yml definitions

2b38684

Update conditions for number to boolean conditions to prevent casting…

4777607

… errors

stash dashboard changes

698ee31

efd6 reviewed Aug 31, 2023

View reviewed changes

adding all dashboards and new screenshots

16d91a8

added fixes to pipelines from PR comments

b69bdad

P1llus marked this pull request as ready for review September 4, 2023 09:57

P1llus requested a review from a team as a code owner September 4, 2023 09:57

added major version bump

a12fd82

kcreddy reviewed Sep 4, 2023

View reviewed changes

efd6 reviewed Sep 4, 2023

View reviewed changes

adding PR comments and updating generated test files

5b6da79

efd6 approved these changes Sep 6, 2023

View reviewed changes

kcreddy approved these changes Sep 6, 2023

View reviewed changes

P1llus merged commit 4844083 into elastic:main Sep 6, 2023

kcreddy mentioned this pull request Sep 6, 2023

[entityanalytics_okta] Add the ECS mapping for entityanalytics_okta package. #7402

Merged

4 tasks

This was referenced Sep 7, 2023

sei: packages potentially using incorrect mustache snippet escaping #7641

Open

m365_defender: fix template snippet escaping behaviour and add event.kind for pipeline errors #7707

Merged

Uh oh!

Conversation

P1llus commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Checklist

Screenshots

elasticmachine commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌐 Coverage report

efd6 left a comment

Choose a reason for hiding this comment

Uh oh!

efd6 Aug 31, 2023

Choose a reason for hiding this comment

P1llus Sep 4, 2023

Choose a reason for hiding this comment

P1llus Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

efd6 Sep 4, 2023

Choose a reason for hiding this comment

P1llus Sep 5, 2023

Choose a reason for hiding this comment

efd6 Sep 5, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

P1llus commented Sep 4, 2023

elasticmachine commented Sep 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

efd6 left a comment

Choose a reason for hiding this comment

kcreddy left a comment

Choose a reason for hiding this comment

Labels

4 participants

P1llus commented Aug 24, 2023 •

edited

Loading

elasticmachine commented Aug 24, 2023 •

edited

Loading

elasticmachine commented Aug 24, 2023 •

edited

Loading

P1llus Sep 4, 2023 •

edited

Loading