-
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
[Filebeat] Update crowdstrike module #20138
Conversation
Pinging @elastic/siem (Team:SIEM) |
evt.Delete("message"); | ||
evt.Delete("host.name"); | ||
dropIfEmpty(evt, "crowdstrike.event.UserIp"); |
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.
Don't think we need this. In the convert it should be "fail_on_error: false", not "ignore_failure: true", then we don't need the dropIfEmpty
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.
Made the change, but are we ok with passing off an empty string in an ip
field? You can take a look at the difference in the approach here: a6d288c
cc: @tonymeehan
First, I heart you. I had started working on this while I was on PTO but haven't had time to get to it since I've been back. I'll provide some comments |
These were my changes:
It's basically two things. First, use fail_on_error to ignore missing fields. Second, convert crowdstrike.event.UTCTimestamp so that it's rendered in the row-rendered view in Elastic Security (it's converted for some reason today in the JSON view but not the row-rendered view). It might also be good to add event.ingested while we are here given #20073 |
@tonymeehan for the Rather than conditionally figuring out which events have unix v. unix ms and potentially missing some, I guess my approach was, "let's just shove all timestamps through this, check to see if they look like Edit: Here's an example of
Here's an example of
|
@andrewstucki gotcha, thanks for double checking. Like we discussed in Slack, I generally don't like guessing the timestamp format and would prefer to make it conditional based on event type (assuming it's at least consistent at the event level), but perhaps in this case for this data and because we don't have direct access to the product to validate whether the format is consistent for each event type, this is the best option we have to render the time correctly in the product. |
So, from the sounds of it, the row renderers better support timestamps that are in string format in their source documents, so I just went ahead and ran all of the timestamps we set through the normalization function and made it also stringify the fields. I can look into adding an |
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.
Needs a Changelog entry, but code changes LGTM
@tonymeehan & @andrewstucki I have access to the product and am happy to provide anything you may need. Attached is a scrubbed set of sample data hope this helps! edit by @gtback: removed the file |
@vi-or-die thanks a bunch, I'll see about adding those into the test harness and it'll help us identify additional field mappings for SIEM compatibility |
processors: | ||
- set: | ||
field: event.ingested | ||
value: '{{_ingest.timestamp}}' |
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.
❤️
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
* Update crowdstrike module (cherry picked from commit 5e9a3a5)
* Update crowdstrike module (cherry picked from commit 5e9a3a5)
* Update crowdstrike module (cherry picked from commit 5e9a3a5)
…ne-2.0 * upstream/master: (41 commits) adding possibility to override content-type checks, it was breaking certain webhooks that is not able to set content-headers at all. Still defaults to application/json (elastic#20232) fix: use a fixed worker type for tests (elastic#20130) [Ingest Manager] Prepare packaging for endpoint and asc files (elastic#20186) [Packetbeat] HTTP: Improve support for 100-continue elastic#15830 (elastic#19349) Increase index.max_docvalue_fields_search to 200 (elastic#20218) [Ingest Manager] Prevent closing closed reader (elastic#20214) [Metricbeat] Use MySQL Host Parser in Query metricset (elastic#20191) Testing: Ignore timestamp from cylance/protect dataset (elastic#20211) [Filebeat] Ignore cylance.protect timestamps while testing (elastic#20207) [CI] remove codecov step (elastic#20102) [docs] Indicate that SYSTEM user is required on Windows to use Endpoint (elastic#20172) Remove f5/firepass rsa2elk fileset (elastic#20160) [Elastic Agent] Improve GRPC stop to be more relaxed. (elastic#20118) Fix fileset field prefixing (elastic#20170) Fix terminating pod autodiscover issue (elastic#20084) Call host parser only once when building light metricsets (elastic#20149) [CI] fix null string with contains (elastic#20182) [Ingest Manager] Fix failing unit tests on windows (elastic#20127) [Filebeat] Update crowdstrike module (elastic#20138) [docs] Add x-pack role to relevant metricsets (elastic#20167) ...
* upstream/7.9: (32 commits) feat(ci): support storing artifacts for PRs in separate dirs (elastic#20282) (elastic#20301) Cisco ASA: Fix message 106100 (elastic#20245) (elastic#20277) [CI] Change upstream reference (elastic#20296) (elastic#20297) [docs] Fix Windows download link for agent (elastic#20258) (elastic#20290) Cherry-pick to 7.9: [docs] Rename release highlights to what's new (elastic#20255) (elastic#20285) Elastic agent on k8s (elastic#19727) (elastic#20262) [Filebeat Module] Defender ATP - Adding dashboard (elastic#20058) (elastic#20093) fix: use a fixed worker type for tests (elastic#20130) (elastic#20247) [Elastic Agent] Fix Windows powershell install service script (elastic#20203) (elastic#20252) [Ingest Manager] Fixed unzip on older windows (elastic#20088) (elastic#20109) adding possibility to override content-type checks, it was breaking certain webhooks that is not able to set content-headers at all. Still defaults to application/json (elastic#20232) (elastic#20237) [Filebeat][Gsuite] Make GSuite docs more clear (elastic#19981) (elastic#20067) Increase index.max_docvalue_fields_search to 200 (elastic#20218) (elastic#20221) Call host parser only once when building light metricsets (elastic#20149) (elastic#20190) [Metricbeat] Use MySQL Host Parser in Query metricset (elastic#20191) (elastic#20212) [Filebeat] Ignore cylance.protect timestamps while testing (elastic#20207) (elastic#20217) [libbeat] Fix write error in ensureWriter.Write (elastic#20112) (elastic#20145) Cherry-pick elastic#20127 to 7.9: Fix failing unit tests on windows (elastic#20180) Remove f5/firepass rsa2elk fileset (elastic#20160) (elastic#20206) Cherry-pick elastic#20138 to 7.9: [Filebeat] Update crowdstrike module (elastic#20177) ...
* Update crowdstrike module
elastic#20177) * [Filebeat] Update crowdstrike module (elastic#20138) * Update crowdstrike module (cherry picked from commit aa58f2e) * Fix up changelog
elastic#20178) * [Filebeat] Update crowdstrike module (elastic#20138) * Update crowdstrike module (cherry picked from commit aa58f2e) * Fix up changelog * Fix merge rendering issues
What does this PR do?
I've been in the crowdstrike module recently anyway and noticed that there was an open issue reporting some parsing errors. I went ahead and just added some fixes for them.
One thing to note--due to normalizing all timestamps to
UNIX_MS
this is technically a breaking change. Do we want to be more conservative about the normalization?Checklist
[ ] I have commented my code, particularly in hard-to-understand areas[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues