-
Notifications
You must be signed in to change notification settings - Fork 429
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
crowdstrike: add support for host info enrichment #8474
Conversation
9042287
to
0457f4b
Compare
🌐 Coverage report
|
/test |
2751001
to
db9ce4a
Compare
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
required: true | ||
show_user: true | ||
title: Enrich Host Metadata | ||
description: Uses data in aidmaster to add host information to events. The aidmaster blob must string "aidmaster" in its path and the FDR Notification Parsing Script must sort events so that aidmaster events appear first in the stream. |
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.
description: Uses data in aidmaster to add host information to events. The aidmaster blob must string "aidmaster" in its path and the FDR Notification Parsing Script must sort events so that aidmaster events appear first in the stream. | |
description: Uses data in aidmaster to add host information to events. The aidmaster blob must contain string "aidmaster" in its path and the FDR Notification Parsing Script must sort events so that aidmaster events appear first in the stream. |
required: true | ||
show_user: true | ||
title: Host Metadata TTL | ||
description: The period of time that host metadata is considered valid for. |
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.
Can you also add valid time units?
target: crowdstrike | ||
- if: | ||
contains: | ||
log.file.path: aidmaster |
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.
Is the field log.file.path
going to exist for S3 input in this case?
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.
Yes.
required: true | ||
show_user: true | ||
title: Host Metadata TTL | ||
description: The period of time that host metadata is considered valid for. |
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.
time units here as well
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 👍🏼
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.
Have you manually tested this using the aws-s3 input?
The fdr datastream offers both s3 and logfile inputs. Limitations in elastic-package prevent us from being able to have two system test infrastructure deployments, so only the logfile input is tested.
If we have to sacrifice the logfile testing to get aws-s3 system tests then let's do it. The aws-s3 input is what most users are running with.
- {{path}} | ||
{{/each}} | ||
scan: | ||
sort: filename | ||
order: asc |
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.
This will be a weak ordering guarantee because the scan can start concurrent readers. You could set harvester_limit: 1
to control concurrency. I don't think the log file input is used in production by anyone so this matters very little.
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.
Thanks, I was worried about that. We can't limit though; making the limit 1 results in a deadlock.
e5a1d0d
to
affaee3
Compare
affaee3
to
8770ae4
Compare
65571d8
to
c305dc4
Compare
Error: error running package system tests: could not complete test run: could not add data stream config to policy: could not add package to policy; API status code = 400; response body = {"statusCode":400,"error":"Bad Request","message":"Package policy is invalid: inputs.aws-s3.streams.crowdstrike.fdr.vars.fdr_parsing_script: Invalid YAML format"}
b621c88
to
c027048
Compare
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. Thanks for getting the system testing setup for the aws-s3 input.
I left some minor suggestions related to the test config.
secret_access_key: "{{AWS_SECRET_ACCESS_KEY}}" | ||
session_token: "{{AWS_SESSION_TOKEN}}" | ||
queue_url: "{{TF_OUTPUT_queue_url}}" | ||
preserve_original_event: true |
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.
I think we should be able to know the expected number of events so can we add the
assert:
hit_count: N
@@ -0,0 +1,9 @@ | |||
input: aws-s3 | |||
wait_for_data_timeout: 20m |
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.
This wait period seems quite high. Once Terraform completes its provisioning an SQS notification should be sent after about 1 minute. I think the default of 10m should be fine.
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.
The 20m value appears to be commonly used, but agreed. Will change.
Package crowdstrike - 1.26.0 containing this change is available at https://epr.elastic.co/search?package=crowdstrike |
Proposed commit message
See title.
Checklist
changelog.yml
file.Author's Checklist
logfiles3 input is tested.How to test this PR locally
Make the following change:
Then:
(
gsed
is GNUsed
so if on macos you will need to install that, is on linuxs/gsed/sed/
in the command above)Then test with
elastic-package
as normal.For convenience (because the AWS setup was not trivial), the TF graph is here.
Related issues
Screenshots