[CrowdStrike] Correct event.action, event.type, and message#19808
[CrowdStrike] Correct event.action, event.type, and message#19808jeff-bb wants to merge 4 commits into
Conversation
Update various sections to utilize event.type over stepping on event.action, allowing message to not be recreated after initial parsing.
ReviewersBuildkite won't run for external contributors automatically; you need to add a comment:
NOTE: https://github.com/elastic/integrations/blob/main/.buildkite/pull-requests.json contains all those details. |
efd6
left a comment
There was a problem hiding this comment.
See #19807 (comment)
Note that this will require a changelog entry and manifest version bump as shown there, and will require that the test expectations be updated.
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
👀 I have started reviewing the PR |
| if: ctx._temp?.isDriver == true | ||
| field: event.action | ||
| value: load | ||
| field: event.type |
There was a problem hiding this comment.
🔴 CRITICAL data_stream/fdr/.../default.yml:1263
Pipeline output changed but expected test fixtures not regenerated
This change alters the emitted documents (drops the message = event_simpleName fallback, moves the driver marker from event.action to event.type, and removes event.action from the registry op_types script), but only default.yml is in this PR — the committed _dev/test/pipeline/*-expected.json fixtures still encode the old output. For example, test-fdr.log-expected.json (KextLoad) expects event.action: "load", event.type: ["start"], and message: "KextLoad"; after this change event.action is left as the simpleName (line 629 still copies crowdstrike.event_simpleName into event.action), event.type becomes the scalar "start", and message is absent. test-fdr-new-events.log-expected.json (RegSystemConfigKeyUpdate, RegOperationType 4) expects event.action: "deletion", which is no longer set. elastic-package test pipeline will fail on these mismatches. The fixtures must be regenerated and committed with this change.
Recommendation:
Regenerate the pipeline test fixtures so they reflect the new output, then commit them alongside default.yml:
cd packages/crowdstrike
elastic-package test pipeline --generate🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| field: event.action | ||
| value: load | ||
| field: event.type | ||
| value: start |
There was a problem hiding this comment.
🟠 HIGH data_stream/fdr/.../default.yml:1264
set event.type=start clobbers categorize-derived type for non-load driver events
This processor runs after the categorize pipeline (invoked at line 554), which already assigns event.type as an array for every driver event, and _temp.isDriver (line 581) is true for all events whose event.category contains driver. Using set with the scalar value start therefore overwrites the correct, categorize-derived type for driver events that are not loads: e.g. KextUnload and DcUsbDeviceDisconnected are mapped to event.type: [end] and DcUsbConfigurationDescriptor/DcUsbHIDDescriptor to [info] in categorize.yml, but this processor clobbers them to "start". It also replaces the ECS array with a scalar string. Because every isDriver event was already classified by categorize.yml, this set only ever overwrites an existing, correct value.
Recommendation:
Drop this processor — categorize.yml already classifies driver events (KextLoad/DriverLoad -> [start], KextUnload -> [end], etc.). If an explicit fallback is still wanted, append instead of overwriting and only when event.type is unset, mirroring the append/allow_duplicates pattern used elsewhere:
- append:
tag: set_event_type_driver_735cfe72
if: ctx._temp?.isDriver == true && ctx.event?.type == null
field: event.type
value: start
allow_duplicates: false🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
This makes sense, with the proposed changes, this becomes duplicative and overwrites desired values.
| tag: set_event_action_526984f8 | ||
| if: ctx.event?.action != null && ctx.event.action.endsWith('Written') && ctx.host?.os?.type == 'windows' | ||
| field: event.action | ||
| - append: |
There was a problem hiding this comment.
🟡 MEDIUM data_stream/fdr/.../default.yml:2242
append event.type=creation produces duplicate values without allow_duplicates:false
The append processor has no allow_duplicates set, so it defaults to true. Its condition (event.action endsWith 'Written') matches the same events that categorize.yml already maps to event.type: [creation] (e.g. GenericFileWritten, PeFileWritten, BmpFileWritten). For those events the result is a duplicated array ["creation", "creation"]. The processor also dropped the previous && ctx.host?.os?.type == 'windows' guard, widening this to all platforms, which increases the set of events that get the duplicate.
Recommendation:
Set allow_duplicates: false so the value is only added when not already present:
- append:
tag: append_event_action_526984f8
if: ctx.event?.action != null && ctx.event.action.endsWith('Written')
field: event.type
value: creation
allow_duplicates: false🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
This probably makes sense to simply remove, classification.yml already covers the use case, and the exact probably already exists with duplication. If a 'catch all' for *Written needs added, it should be placed in classification.yml as well.
This is already handled elsewhere.
|
👀 I have started reviewing the PR |
script_set_event_type_29345ceb script_set_event_action_and_type_29345ceb
|
👀 I have started reviewing the PR |
efd6
left a comment
There was a problem hiding this comment.
This will need to have test updated (see my comment in your other PR about how to do this), and changelog and manifest updated (see my comment at the issue).
From my investigation, you will also need to add
diff --git a/packages/crowdstrike/data_stream/fdr/elasticsearch/ingest_pipeline/categorize.yml b/packages/crowdstrike/data_stream/fdr/elasticsearch/ingest_pipeline/categorize.yml
index 85e9253eda..e4e109d379 100644
--- a/packages/crowdstrike/data_stream/fdr/elasticsearch/ingest_pipeline/categorize.yml
+++ b/packages/crowdstrike/data_stream/fdr/elasticsearch/ingest_pipeline/categorize.yml
@@ -1201,6 +1201,7 @@ processors:
outcome: success
type:
- change
+ - creation
MachOHeaderParsed:
category:
- file|
@jeff-bb I have sent a counterproposal (essentially what I posted in the issue, but with tests). |
Proposed commit message
Update various sections to utilize event.type over stepping on event.action, allowing message to not be recreated after initial parsing.
Related issues