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

Add integration for Recorded Future threat intel #2757

Merged
merged 7 commits into from
Mar 16, 2022
Merged

Add integration for Recorded Future threat intel #2757

merged 7 commits into from
Mar 16, 2022

Conversation

adriansr
Copy link
Contributor

@adriansr adriansr commented Mar 1, 2022

What does this PR do?

Adds a new integration to collect threat indicators from Recorded Future.

This supersedes the existing (recalled) Filebeat integration.

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.

Author's Checklist

  • Review version number (0.1.0) and beta tag.
  • Review current approach as to data_streams.
  • Missing dashboard.

See comment below.

Related issues

Screenshots

Screen Shot 2022-03-01 at 21 31 36

Screenshot 2022-03-01 at 21-32-09 Add integration - Recorded Future - Integrations - Elastic

@adriansr
Copy link
Contributor Author

adriansr commented Mar 1, 2022

Note to reviewers.

I have a few doubts:

  • We're making this beta to start with, which makes sense. I don't think it will need too many changes before becoming ga. I was unusure what the right first version number will be in this case. 0.1.0 looks more suited for an experimental package. 1.0.0 is better reserved for when it goes ga. How does 0.9.0 sound?

  • The functionality of this package is a bit different. Due to single input limitations and Recorded Future separating the entities to ingest in different API endpoints (url, ip, domain and hash), the (common?) use case of ingesting more than one kind of data will require setting up multiple integrations. I've considered separate datastreams for each kind, but it will result in a lot of repetition (sample pipelines, same fields, same tests...). WDYT about this choice?

  • After discussion with @P1llus, we want to take our time revamping the original Filebeat dashboard to align with the other ti_* packages. For now I think it's better to merge the initial version without a dashboard.

@elasticmachine
Copy link

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

@elasticmachine
Copy link

elasticmachine commented Mar 1, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-16T08:05:04.523+0000

  • Duration: 15 min 23 sec

Test stats 🧪

Test Results
Failed 0
Passed 11
Skipped 0
Total 11

🤖 GitHub comments

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

  • /test : Re-trigger the build.

@adriansr adriansr requested review from P1llus and a team March 1, 2022 20:53
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Nice.

@@ -0,0 +1,29 @@
name: ti_recordedfuture
title: Recorded Future
version: 0.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the right choice FWIW. If it were experimental I'd say 0.0.1, though with semver<1.0.0 the semantics are essentially free.

@@ -0,0 +1,852 @@
{
"expected": [
null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this null expected? (also in some other inputs below: test-hash-default.log-expected.json, test-ip-default.log-expected.json and test-url-default.log-expected.json).

Copy link
Member

Choose a reason for hiding this comment

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

I assume the null is for the header line and that line is dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, is the header line. This only happens with CSV input from file.

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.

As always, a well documented pipeline 👍


Alternatively, it's also possible to use the integration to fetch custom Fusion files
by supplying the URL to the CSV file as the _Custom_ _URL_ configuration option.

Copy link
Member

Choose a reason for hiding this comment

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

Are there any links you can add to publicly available Recorded Future docs about the API or schema?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately the API docs now require signing up to RecordedFuture's support portal.

https://support.recordedfuture.com/hc/en-us/articles/115000897248

@@ -0,0 +1,852 @@
{
"expected": [
null,
Copy link
Member

Choose a reason for hiding this comment

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

I assume the null is for the header line and that line is dropped.

field: threat.feed.name
value: "Recorded Future"
#
# TODO: Add dashboard
Copy link
Member

Choose a reason for hiding this comment

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

What's the plan for addressing this TODO? Separate PR?

And given that there is a commented out constant_keyword version with a static value in the fields.yml, do we need this one at all?

Copy link
Member

Choose a reason for hiding this comment

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

The commented values could be moved to base-fields.yml, similar to how the other TI packages does it, but they should not be added before the dashboards as they are required, else it will break UI components

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So for now we can leave as is and then add the dashboard in a follow up PR, is that ok?

- event.dataset
- threat.indicator.type
- json.Name
target_field: "_id"
Copy link
Member

@andrewkroh andrewkroh Mar 10, 2022

Choose a reason for hiding this comment

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

Do we expect to receive updates for the data and discard it due to the append only nature of data streams? I just want to understand the expected behavior and maybe document it in the pipeline comments.

Copy link
Member

Choose a reason for hiding this comment

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

I think mostly all the TI sources has it, before it was for appending indeed, but there are scenarios which could result in duplicates, so it's just to make sure values are not stored twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this seems useless in data streams, unless the Kibana alerting side somehow uses the ID to deduplicate.

Copy link
Member

@P1llus P1llus left a comment

Choose a reason for hiding this comment

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

I don't have anything extra to add compared to the other reviewers. I added some comments into a few review notes by Andrew

@adriansr adriansr requested a review from a team as a code owner March 14, 2022 16:46
Otherwise the fingerprint processor calculating `id` field doesn't have
access to this constant_keyword field as it is not yet set in the
document.
@adriansr adriansr merged commit 28fd9fa into elastic:main Mar 16, 2022
@adriansr adriansr deleted the add_recordedfuture_ti branch March 16, 2022 10:30
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
Otherwise the fingerprint processor calculating `id` field doesn't have
access to this constant_keyword field as it is not yet set in the
document.
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.

Recorded Future
6 participants