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

[RFC] Create Threat Fieldset - Stage 3 Proposal #1480

Merged
merged 18 commits into from
Aug 19, 2021
Merged

[RFC] Create Threat Fieldset - Stage 3 Proposal #1480

merged 18 commits into from
Aug 19, 2021

Conversation

peasead
Copy link
Contributor

@peasead peasead commented Jun 28, 2021

  • Have you signed the contributor license agreement? Yes
  • Have you followed the contributor guidelines? Yes
  • For proposing substantial changes or additions to the schema, have you reviewed the RFC process? Yes
  • If submitting code/script changes, have you verified all tests pass locally using make test? Yes
  • If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes? Yes
  • Is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed. Yes
  • Have you added an entry to the CHANGELOG.next.md? NA

Stage 3 criteria:

  • Opened pull request for this candidate revising the existing candidate
  • Included multiple real world example source documents
  • Existing or newly raised questions and concerns are addressed
  • No objections from sponsor, ECS team, or subject matter experts

Preview of markdown proposal doc

@peasead peasead mentioned this pull request Jul 1, 2021
@peasead peasead marked this pull request as draft July 1, 2021 14:47
@peasead peasead marked this pull request as ready for review July 8, 2021 19:30
@peasead peasead marked this pull request as draft July 8, 2021 19:30
@peasead peasead marked this pull request as ready for review July 8, 2021 19:35
Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

Now that more implementation work and testing has been done, do we have any resolution we can capture under Concerns?

@ebeahan ebeahan requested a review from rylnd July 15, 2021 21:34
@peasead
Copy link
Contributor Author

peasead commented Jul 29, 2021

For # 1 under Concerns WRT to malware name and type, can we use the threat.software.type and threat.software.name fieldsets now?

@rylnd or @dcode under Concerns, is there anything that can be resolved WRT # 2?

@peasead
Copy link
Contributor Author

peasead commented Aug 2, 2021

For # 1 under Concerns WRT to malware name and type, can we use the threat.software.type and threat.software.name fieldsets now?

@rylnd or @dcode under Concerns, is there anything that can be resolved WRT # 2?

Just pinging on this @rylnd / @dcode

@peasead peasead marked this pull request as draft August 3, 2021 19:37
@@ -291,6 +310,8 @@ Stage 1: Identify potential concerns, implementation challenges, or complexity.
-->

1. How to best represent malware{name,family,type}. Current proposal is to use `threat.indicator.classification` to describe threat delivery (Hacktool etc.) and family name.
- This can be represented through the use of the [`threat.software.*`](https://www.elastic.co/guide/en/ecs/master/ecs-threat.html) fields.
- Awaiting [approval](https://github.com/elastic/ecs/pull/1480#issuecomment-889312434) of this recommendation.
1. Field types (Ref: https://github.com/elastic/ecs/pull/1127#issuecomment-776126293)
- `threatintel.indicator.*` (Filebeat module) will be normal field type and will be deprecated when nested field types are better supported in Kibana
- `threat.indicator.*` (actual threat ECS fieldset) will be nested now and used for enriched doc
Copy link
Contributor

Choose a reason for hiding this comment

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

@peasead I don't think this is still true; the plan for threat.indicator is to keep it as a normal field, as is already the case with threatintel.indicator. RE the migration below, I think the only change on the data shipper side is to rename threatintel.indicator to threat.indicator.

threat.enrichments is the nested field that will cover the enrichment use case; the caveats about kibana support still apply there for Detection Alerts and users manually adopting the threat.enrichments fields, but it should not impact data shippers at this time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, I believe threat.enrichments bypasses the nested support issue by allowing:

  1. Data shippers to generate normal threat.indicator fields
  2. Enrichment mechanisms to populate nested threat.enrichments fields

Users wishing to ingest their own CTI data into indicator documents to be consumed within the detection engine can either:

  1. Populate normal threatintel.indicator fields, a la the current threatintel filebeat modules, as that is the current standard and the default for indicator match rules (for 7.14)
  2. Populate normal threat.indicator fields, as that will be the default enrichment path for indicator match rules in 7.15

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Roger.

I'll update the Concern.

@ebeahan FYI ☝️

@peasead peasead marked this pull request as ready for review August 9, 2021 17:27
@peasead
Copy link
Contributor Author

peasead commented Aug 9, 2021

@ebeahan we're ready for review

Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

I identified a couple of small items, but otherwise LGTM.

@ebeahan
Copy link
Member

ebeahan commented Aug 13, 2021

@devonakerr Would you provide your final review here as sponsor?

@peasead peasead requested a review from rylnd August 16, 2021 15:23
@peasead
Copy link
Contributor Author

peasead commented Aug 18, 2021

We're ready for a final review.
@rylnd
@devonakerr
@dcode

Copy link

@devonakerr devonakerr left a comment

Choose a reason for hiding this comment

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

Looks good, remarkable how far we've come with the schema in such a short period.

@peasead peasead merged commit 2d0a721 into elastic:master Aug 19, 2021
@rylnd rylnd mentioned this pull request Aug 20, 2021
4 tasks
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.

4 participants