-
Notifications
You must be signed in to change notification settings - Fork 458
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
[ti_cif3] Add New TI integration for the Collective Intelligence Framework v3 #3839
Conversation
💚 CLA has been signed |
/test |
Hey @mdavis332, as mentioned in #3904 (comment), please check that you are using the same email address in your commits that you used to sign the CLA. |
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
What I really like about this is the inclusion of Threat Fox as a Provider. It's a high-quality and contextual feed. |
- rename: | ||
field: cif3.indicator | ||
target_field: tls.client.ja3 | ||
ignore_missing: true | ||
if: "ctx.cif3?.itype == 'md5' && ctx.cif3?.tags.contains('ja3')" |
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.
Looking at field reuse for the Threat ECS fieldset, I think this is a gap that needs to be addressed by adding tls.
as a valid reuse field for threat.*
.
Ideally, we'd want this to be something like threat.indicator.tls.client.ja3
.
@ebeahan would we be able to make a parallel effort in adding TLS as a valid reuse field and adjusting this pipeline to be threat.indicator.tls.client.ja3
or do we need to adjust ECS first?
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.
@peasead thanks so much for carefully looking at the integration. Although I'm not elastic staff, I agree with the suggestion of adding the TLS field for reuse under threat.indicator
. I had the same thought about threat.indicator.network.cidr
. Although it's not currently on the main ECS-threat reuse section, I went ahead and used that network.cidr
field under threat.indicator
due to having read through this related convo: elastic/beats#29949
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.
Sure thing.
This all looks really good and we'll wait from @ebeahan to see what his thoughts are on the ECS side.
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.
Apologies - missed the mention here until now.
No objection on the naming for threat.indicator.tls.client.ja3
. Since the field doesn't exist in ECS, it will need defining in the integration itself.
If there's interest to add tls.*
field reuse under threat.indicator.*
, I think it's a small enough addition it can be suggested in a PR vs. through an ECS RFC.
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.
Hi @ebeahan,
Since the field doesn't exist in ECS, it will need defining in the integration itself.
Would that involve tweaking data_stream/feed/fields/ecs.yml
- external: ecs
name: tls.client.ja3
to
- external: ecs
name: threat.indicator.tls.client.ja3
?
I think it's a small enough addition it can be suggested in a PR
Is that a suggestion for me or elastic staff? I'm not as familiar with https://github.com/elastic/ecs. I assume it would need to be added somewhere in https://github.com/elastic/ecs/blob/main/schemas/threat.yml, but I'm unsure.
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.
It would need to be an explicit definition, analogous to this
- name: source.geo.location | |
level: core | |
type: geo_point | |
description: Longitude and latitude. |
This was done for geo.location
before it was included in ECS formally as a geo_point
type.
What needs to be added is just the type and the relevant details for the ja3 field from https://www.elastic.co/guide/en/ecs/current/ecs-tls.html#field-tls-client-ja3.
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.
just so there's no unmet expectations, ThreatFox isn't inherently part of this integration or one of the default CIFv3 ingestion rules, but since it's public info and a worthwhile provider of info, I included it as our pipeline test example :) It's definitely possible to configure as a an ingestion rule in csirtg-smrt, the ingest module of CIFv3. |
Because I'm looking to use this - what is the current status of this integration? and what still needs to happen? |
hey @eriroley, I think we may be waiting on @ebeahan to give his thoughts on the ECS side of things. Not sure what else may be required. |
To anyone from the elastic team: just checking in to see if there's anything I can do to help move forward here. |
Checking internally on the status. |
Hi @peasead, just my weekly checkin :) There's an event coming up in the middle of September for which I'd love this to be ready to install by end users. If it's not possible to make it into the EPM by then, is there an easy way for end-users to download the integration and locally load it? Kinda like installing/side-loading a local plugin from zip or something? Thanks for checking, and nice work on the recent blog post for the elastic container project. |
@mdavis332 apologies for the delay on getting your PR reviewed. We're working through a backlog of new integration PR's at the moment, so it's taking a little longer than expected. We'll certainly aim to have it published in time for your mid-September event. @P1llus do you have the bandwidth to review this PR over the next week or so? No worries if not, can ask someone else on the team to review. |
packages/ti_cif3/data_stream/feed/_dev/test/pipeline/test-common-config.yml
Outdated
Show resolved
Hide resolved
packages/ti_cif3/data_stream/feed/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/ti_cif3/data_stream/feed/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
packages/ti_cif3/data_stream/feed/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
updated real PR link Co-authored-by: Dan Kortschak <[email protected]>
Add support for IPV6 addresses with ports by looking for surrounding brackets, e.g., `http://[2001:d34d:b33f:0000:0000:0000:d34d:b33f]:8080`
Hi @efd6, thanks so much for your time doing a thorough review. I believe I've addressed all of your review feedback and left comments on each with referenced commits. Please let me know if there's anything else to adjust at this time. Thank you again! |
/test |
The README.md will need to be rebuilt. |
Done via commit 57a110a. |
/test |
packages/ti_cif3/data_stream/feed/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
Co-authored-by: Dan Kortschak <[email protected]>
/test |
packages/ti_cif3/data_stream/feed/_dev/test/pipeline/test-cif3-sample.ndjson.log
Outdated
Show resolved
Hide resolved
/test |
🌐 Coverage report
|
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, but please wait for @ebeahan for comment on the ECS issue.
Thanks |
Thanks again, Dan, for all your time reviewing and working through the CI stuff. |
Hi @jamiehynds, we're coming up on that mid-September timeframe, so just wanted to check back in this week. I appreciate everyone that's contributed to moving this PR forward. |
@efd6 is this PR ok to merge now? The only outstanding item was the TI ECS query, which Eric commented on and @mdavis332 has addressed. Thanks! |
🚀 Thanks @mdavis332 |
What does this PR do?
Checklist
changelog.yml
file.How to test this PR locally
elastic-package stack up -v -d
Screenshots