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

New input for Crowdstrike Falcon events #16988

Merged
merged 30 commits into from
Mar 31, 2020
Merged

New input for Crowdstrike Falcon events #16988

merged 30 commits into from
Mar 31, 2020

Conversation

tonymeehan
Copy link

What does this PR do?

This adds a new input for Crowdstrike Falcon events forwarded by Crowdstrike's SIEM forwarder. This input uses the default JSON output format from the SIEM forwarder.

Why is it important?

We've had several users asks us for adding support for Crowdstrike Falcon in ECS. We strive to be data agnostic to help enable our SOC users aggregate all of their data in our SIEM. Using some log data provided by our users, I've put this new module together to ingest the data and convert many fields into ECS (including categorization).

Checklist

  • My code follows the style guidelines of this project
  • 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 files
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

mage update build integTest

Screenshots

crowdstrike mov

Proof this works

[success] 0.85% test_xpack_modules.XPackTest.test_fileset_file_008_crowdstrike: 2.7708s
----------------------------------------------------------------------
Ran 135 tests in 326.929s

OK

I also ran make docs to validate the documentation looked good.

Other comments

I'll squash before merging.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@tonymeehan tonymeehan requested review from tsg and andrewkroh March 12, 2020 21:09
@tonymeehan
Copy link
Author

Looking into CI errors. Need to run make check and mage fmt update

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.

Looking good :-)

couple general questions/suggestions:

  • any value in geoip on ip addrs?
  • if event has ips, users or hashes probably a good idea to add those to the related fields

specific questions in-line.

Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

This is a great idea!

This is just a preliminary review, here's a bunch of comments and suggestions :-)

Broader discussion point: as I note in one of my comments, this seems to be a log that mixes their service's audit logs with actual endpoint detection events. Both are worth capturing, of course. But I wonder if we should find a way to separate them out in two different event.dataset, as their meaning & semantics are pretty different.

Doing so would help clarify different handling of various things like host.name, how to fill event.action, etc; that have been raised by me and @leehinman.


evt.Put("event.action", tactic + "_" + technique)
evt.Put("event.kind", "alert")
evt.Put("event.type", "info")
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed. @leehinman, this is how to make it into an array, correct?

Suggested change
evt.Put("event.type", "info")
evt.Append("event.type", "info")

@@ -0,0 +1,19 @@
type: log
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's worth also supporting the syslog input here? https://www.crowdstrike.com/blog/tech-center/integrate-with-your-siem/ seems to suggest that it's possible for the Connector to send date directly to a syslog server?

Copy link
Author

Choose a reason for hiding this comment

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

It would be easy to add the config option for syslog and maybe not make it the default choice. But my concern is we can't test it to validate that the output is identical to the file output. I'm on the fence but can be convinced to add it if it's not the default choice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, makes sense. I think we can leave it out and add it if it's being requested. Then we have a way to validate it.

@andrewkroh
Copy link
Member

It looks like CI is much more happy now. The failures are from flakey tests and appear unrelated to your changes.

@tonymeehan
Copy link
Author

It looks like CI is much more happy now. The failures are from flakey tests and appear unrelated to your changes.

Yep, thanks @andrewkroh. I was just rebuilding to see if that was the case. I'll start working on the recommendations tonight.

@tonymeehan
Copy link
Author

Thanks for the feedback.

PR Feedback Addressed

  • Split up the filesets into two components, falcon_endpoint and falcon_audit.
  • Added additional ECS 1.5 fields
  • Fixed some bugs (e.g. event.type and event.category are array fields)

PR Feedback Not Addressed

  • Geo IP -- two reasons -- most endpoint IPs will be private, geo data won't make sense here. And the the other reason was I wanted to avoid installing an ingest pipeline for our users that have Logstash setup between their Beats and the stack.
  • Setting enabled to false in config.yml (rationale explained above)

Screen Shot 2020-03-26 at 9 09 58 AM

Screen Shot 2020-03-26 at 9 10 37 AM

@tonymeehan
Copy link
Author

jenkins retest this please

@tonymeehan
Copy link
Author

I've been able to address most of the PR feedback (thanks again, everyone). I think we're ready for some LGTMs unless anyone has any additional feedback/changes.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

This looks pretty good! Two small requests:

  • The docs need changed to reflect there's only one fileset.
  • Please add an entry to the CHANGELOG.next.asciidoc file in the root of the repo under the Filebeat/Added section.

x-pack/filebeat/module/crowdstrike/_meta/docs.asciidoc Outdated Show resolved Hide resolved

This is the filebeat module for the Crowdstrike Falcon using the Falcon https://www.crowdstrike.com/blog/tech-center/integrate-with-your-siem[SIEM Connector]. This module collects this data, converts it to ECS, and ingests it to view in the SIEM. By default, the Falcon SIEM connector outputs JSON formated Falcon Streaming API event data in JSON format.

This module segments events forwarded by the Falcon SIEM connector into two datasets for endpoint data and Falcon platform audit data.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This module segments events forwarded by the Falcon SIEM connector into two datasets for endpoint data and Falcon platform audit data.

Copy link
Author

Choose a reason for hiding this comment

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

I was going to keep this since we're still doing this (two datasets), just from the same fileset. That okay?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, that's interesting. I didn't notice that the module was setting the event.dataset values. We've never had a case AFAIK where a fileset produced an event.dataset value other than the default of {module}.{fileset}. I don't think this will cause any issues. I think it would be good to callout the event.dataset values that it produces in this paragraph.

x-pack/filebeat/module/crowdstrike/_meta/docs.asciidoc Outdated Show resolved Hide resolved
@leehinman
Copy link
Contributor

Changes look really good.

Looks like you need to run "mage fmt update" and push changes though.

@tonymeehan tonymeehan merged commit 4e02957 into elastic:master Mar 31, 2020
@tonymeehan tonymeehan deleted the feature-crowdstrike-module branch March 31, 2020 13:31
@tonymeehan
Copy link
Author

Thanks for all the help @leehinman @andrewkroh @webmat and @tsg!

tonymeehan pushed a commit that referenced this pull request Apr 21, 2020
…17770)

Backport new input for Crowdstrike Falcon events (#16988)
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.

6 participants