-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Security Solution][CTI] Update legacy CTI signals to latest ECS threat fields #107988
Conversation
The nested threat.indicator mappings were experimental, and replaced by threat.enrichmentsin ECS 1.10. While these fields are also experimental, they fix the conflict between CTI data's normal threat.indicator mappings.
event.* is no longer nested within here; it was determined that event fields were not relevant to enrichment. All relevant ECS fieldsets (file, pe, etc) are now nested under threat.enrichments.
This test is a snapshot of the actual mappings applied by our templates. Looks good to me!
We now have two threat fields we care about for CTI, for legacy and official ECS.
They're still queryable by threat.indicator, meaning that any existing dashboards will still work.
* Defines reindex script to move things around * Adds integration tests to make sure the migration and new mappings work * Need to test a few more things and verify corner cases * Need to extract some helpers from tests
Marshall bumped to 55, giving us 10 versions for 7.14.x updates. However, devs would not otherwise roll over and get my mapping updates without destroying their signals index and rebuilding (which is also not the same thing, exactly), so this trades having one higher signals version for a more streamlined dev workflow.
We only attempt to migrate legacy enrichments if the document: * is a signal from an indicator match rule * has a `threat.indicator` field * does not have a `threat.enrichments` field
Tests a few more pieces of the resulting document, giving more confidence that it's the correct transformation (and mappings). This also modifies/anonymizes the data that was originally generated on a work machine.
This was for when these tests were driven via the UI; the API is more responsive and now synchronization is currently needed here, beyond the 200 responses.
These fields are in ECS 1.11.
We bumped the version previously, causing this test to become outdated.
These were copied from the security_solution plugin. I updated those, but neglected to update these. Until there's a better mechanism for deduplication here, I'm going to kick the can and update both for now.
* indicator match rule logic * we now simply copy from the specified indicator path, and place that in `threat.enrichments.indicator` * event enrichment API logic * We were previously returning fields from `indicator.*`, we now include the `indicator.*` suffix in order to be more consistent with the sibling `matched.*` fields * row renderer logic * removal of dataset * updates relevant to API changes above
We want to link the reference field, not a `first_seen` field.
Prior to this change we would display e.g. `threatintel.indicator.foo` for investigation enrichment fields. Now that the structure has changed slightly and we return both `indicator.*` and `matched.*` fields for existing enrichents, we want to display investigation enrichment similarly.
Now that we've updated our enrichment logic, we need to update our enrichment tests.
We were not requesting the necessary fields for our row renderer, since these constants (specifically CTI_ROW_RENDERER_FIELDS) now exist in both security_solution and the timelines plugin. I had updated the former, but only the latter is actually used.
These fields are prefixed with `indicator` now because: 1. This data pertains to the indicator, not the match per se 2. The actual field is prefixed with indicator (or, it at least specifies an indicator in the case of a custom threat index (via threat_indicator_path))
Ths one involved updating a mock in another package.
When we query indicators for enrichment matches, the current expectation is that we'll be querying 7.14 filebeat modules, which have an indicator path of 'threatintel.indicator'. The only place that matters on the UI is on the threat intel panel, where these indicators come back with such a prefix. This change has one behavior: it brings back the `provider` field on the Alert summary tab for queried enrichments from filebeat modules.
Pinging @elastic/security-solution (Team: SecuritySolution) |
@elasticmachine merge upstream |
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 was wondering if x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/field_maps/cti.ts
should be updated as well as a part of this change. Change looks good, running the code as described in the scenario to give out the final approval
*/ | ||
export const getShimmedIndicatorValue = (enrichment: CtiEnrichment, field: string) => | ||
getEnrichmentValue(enrichment, field) || | ||
getEnrichmentValue(enrichment, `${DEFAULT_INDICATOR_SOURCE_PATH}.${field}`); | ||
getEnrichmentValue(enrichment, `threatintel.${field}`) || |
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.
looks like we are checking the legacy version first - assuming that it's either one or the other, the order shouldn't matter, but in general it makes sense to me to check if the most recent version exists (ECS 1.11), and default to legacy if that is not the 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.
That's a good point; I had chosen to check the legacy one first because none of our data shippers actually generate threat.indicator
yet. If/when that happens, the following also need to change:
- Prebuilt indicator rule logic
- default indicator rule
threat_indicator_path
// and making this code dynamic on an arbitrary path would introduce several | ||
// new issues. | ||
const existingIndicatorValue = get(signalHit._source, 'threat.indicator') ?? []; | ||
const existingIndicatorValue = get(signalHit._source, 'threat.enrichments') ?? []; |
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 this an indicator at this point or an enrichment? we could consider renaming this variable to existingEnrichmentsValue
and the one in the next line existingEnrichments
for easier understanding
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.
Updated this (and a few other related variables/methods) in f52c6c4
event: ecsMapping.mappings.properties.event, | ||
}, | ||
enrichments: { | ||
...otherMapping.mappings.properties.threat.properties.enrichments, |
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 was wondering why we are maintaining this piece separately from ecsMapping
- shouldn't enrichments
be reading from ecsMapping
(instead of otherMapping
) where ecsMapping.mappings.properties.threat.properties.enrichments
is up to date with ECS 1.11?
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.
That's a good point; I had left these separate since they are still beta, but turns out that beta fields appear just like regular fields in the published mappings: https:/elastic/ecs/blob/1.11/generated/elasticsearch/7/template.json so I agree that these should be in ecsMapping
for consistency.
I had wanted to do a separate PR to update ALL mappings to 1.11 (which this PR doesn't do), so we can address this in that effort.
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.
was able to verify that the feature works as intended (note - there seemed to be an error mapper_parsing_exception: Invalid [path] value [signal.original_event.reason] for field alias
which I was able to bypass with the following query
PUT .siem-signals-ece-default/_mapping
{
"properties": {
"signal": {
"properties": {
"original_event": {
"properties": {
"reason": {
"type": "text"
}
}
}
}
}
}
}
event renderer looks good with new alert
threat intel tab looks good with new alert
one concern I had was that this change reverts elastic/security-team#951. The removal of the event fields looked intentional, however we are removing a feature that we have previously provided to our users, which doesn't sound ideal to me. if there is a way to keep around the event information (which our users gave appreciative feedback of) I think that would be ideal.
@ecezalp the Our indicator queries (for dashboards, overview card) can continue to use |
I wasn't aware that they weren't deemed to be particularly useful - I thought that the feature was specifically requested! That being said I like the idea of relying on the schema exclusively in general, so I have no qualms about the removal. Thanks for letting me know! |
…terminology Indicators come from a CTI index. Enrichments are the application of indicator data to other documents, and contain both indicator fields and matched context.
enrichment.indicator = indicator; | ||
enrichment.indicator.reference = indicator.event?.reference; | ||
enrichment.matched = indicator.matched; | ||
enrichment.indicator.remove("matched"); |
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.
Should enrichment.indicator.event
be removed 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.
Nvm, just saw the comment above explaining the logic
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 opted to leave it in case existing CTI users want/need that info (albeit unmapped).
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.
Migration and schema changes LGTM
Conflicts: x-pack/plugins/security_solution/common/constants.ts
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @rylnd |
…at fields (elastic#107988) * WIP: Adding integration test * Replace threat.indicator mappings with threat.enrichments mappings The nested threat.indicator mappings were experimental, and replaced by threat.enrichmentsin ECS 1.10. While these fields are also experimental, they fix the conflict between CTI data's normal threat.indicator mappings. * Add threat.enrichments mappings to our signals template mappings event.* is no longer nested within here; it was determined that event fields were not relevant to enrichment. All relevant ECS fieldsets (file, pe, etc) are now nested under threat.enrichments. * Update snapshot with newest threat.enrichments mappings This test is a snapshot of the actual mappings applied by our templates. Looks good to me! * Update ECS types to match latest We now have two threat fields we care about for CTI, for legacy and official ECS. * Add a basic test for behavior of legacy enriched signals. They're still queryable by threat.indicator, meaning that any existing dashboards will still work. * WIP: First pass at a data migration for CTI signals * Defines reindex script to move things around * Adds integration tests to make sure the migration and new mappings work * Need to test a few more things and verify corner cases * Need to extract some helpers from tests * Bump our template version to ensure devs roll over Marshall bumped to 55, giving us 10 versions for 7.14.x updates. However, devs would not otherwise roll over and get my mapping updates without destroying their signals index and rebuilding (which is also not the same thing, exactly), so this trades having one higher signals version for a more streamlined dev workflow. * More robust guard against data migration We only attempt to migrate legacy enrichments if the document: * is a signal from an indicator match rule * has a `threat.indicator` field * does not have a `threat.enrichments` field * Minor reorder of operations to make logic clearer * Add more assertions around our signals data migration Tests a few more pieces of the resulting document, giving more confidence that it's the correct transformation (and mappings). This also modifies/anonymizes the data that was originally generated on a work machine. * Remove outdated note This was for when these tests were driven via the UI; the API is more responsive and now synchronization is currently needed here, beyond the 200 responses. * Fix typo in comment These fields are in ECS 1.11. * Update snapshot test We bumped the version previously, causing this test to become outdated. * Update ECS typings in timelines plugin These were copied from the security_solution plugin. I updated those, but neglected to update these. Until there's a better mechanism for deduplication here, I'm going to kick the can and update both for now. * Update enrichments logic to read/write from threat.enrichments * indicator match rule logic * we now simply copy from the specified indicator path, and place that in `threat.enrichments.indicator` * event enrichment API logic * We were previously returning fields from `indicator.*`, we now include the `indicator.*` suffix in order to be more consistent with the sibling `matched.*` fields * row renderer logic * removal of dataset * updates relevant to API changes above * Fix logical error in generating links from indicator fields We want to link the reference field, not a `first_seen` field. * Always include the indicator prefix in first-party indicator fields Prior to this change we would display e.g. `threatintel.indicator.foo` for investigation enrichment fields. Now that the structure has changed slightly and we return both `indicator.*` and `matched.*` fields for existing enrichents, we want to display investigation enrichment similarly. * Update indicator match rule integration tests Now that we've updated our enrichment logic, we need to update our enrichment tests. * Remove unused translation * Update example row renderer data for enriched alerts * Update parallel CTI constants to get our CTI row renderer working We were not requesting the necessary fields for our row renderer, since these constants (specifically CTI_ROW_RENDERER_FIELDS) now exist in both security_solution and the timelines plugin. I had updated the former, but only the latter is actually used. * Update CTI enrichment UI tests * Update prepackaged threat timeline template with new threat fields Also bumps the timelineTemplateVersion. * Update Indicator Match rule tests These needed three things: * Update to timeline template (see previous commit) * Changing expectations from `threat.indicator` to `threat.enrichments` * Update row renderer expectation to exclude dataset * Update mock data with newest CTI enrichment fields * Fix assertion on our threat details These fields are prefixed with `indicator` now because: 1. This data pertains to the indicator, not the match per se 2. The actual field is prefixed with indicator (or, it at least specifies an indicator in the case of a custom threat index (via threat_indicator_path)) * Update test data and tests for our field parsing helpers * Update more event-parsing tests Ths one involved updating a mock in another package. * Modify our helper function to support old filebeat indicators When we query indicators for enrichment matches, the current expectation is that we'll be querying 7.14 filebeat modules, which have an indicator path of 'threatintel.indicator'. The only place that matters on the UI is on the threat intel panel, where these indicators come back with such a prefix. This change has one behavior: it brings back the `provider` field on the Alert summary tab for queried enrichments from filebeat modules. * Update variable and method names to be more consistent with internal terminology Indicators come from a CTI index. Enrichments are the application of indicator data to other documents, and contain both indicator fields and matched context. Co-authored-by: Kibana Machine <[email protected]>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…at fields (#107988) (#108628) * WIP: Adding integration test * Replace threat.indicator mappings with threat.enrichments mappings The nested threat.indicator mappings were experimental, and replaced by threat.enrichmentsin ECS 1.10. While these fields are also experimental, they fix the conflict between CTI data's normal threat.indicator mappings. * Add threat.enrichments mappings to our signals template mappings event.* is no longer nested within here; it was determined that event fields were not relevant to enrichment. All relevant ECS fieldsets (file, pe, etc) are now nested under threat.enrichments. * Update snapshot with newest threat.enrichments mappings This test is a snapshot of the actual mappings applied by our templates. Looks good to me! * Update ECS types to match latest We now have two threat fields we care about for CTI, for legacy and official ECS. * Add a basic test for behavior of legacy enriched signals. They're still queryable by threat.indicator, meaning that any existing dashboards will still work. * WIP: First pass at a data migration for CTI signals * Defines reindex script to move things around * Adds integration tests to make sure the migration and new mappings work * Need to test a few more things and verify corner cases * Need to extract some helpers from tests * Bump our template version to ensure devs roll over Marshall bumped to 55, giving us 10 versions for 7.14.x updates. However, devs would not otherwise roll over and get my mapping updates without destroying their signals index and rebuilding (which is also not the same thing, exactly), so this trades having one higher signals version for a more streamlined dev workflow. * More robust guard against data migration We only attempt to migrate legacy enrichments if the document: * is a signal from an indicator match rule * has a `threat.indicator` field * does not have a `threat.enrichments` field * Minor reorder of operations to make logic clearer * Add more assertions around our signals data migration Tests a few more pieces of the resulting document, giving more confidence that it's the correct transformation (and mappings). This also modifies/anonymizes the data that was originally generated on a work machine. * Remove outdated note This was for when these tests were driven via the UI; the API is more responsive and now synchronization is currently needed here, beyond the 200 responses. * Fix typo in comment These fields are in ECS 1.11. * Update snapshot test We bumped the version previously, causing this test to become outdated. * Update ECS typings in timelines plugin These were copied from the security_solution plugin. I updated those, but neglected to update these. Until there's a better mechanism for deduplication here, I'm going to kick the can and update both for now. * Update enrichments logic to read/write from threat.enrichments * indicator match rule logic * we now simply copy from the specified indicator path, and place that in `threat.enrichments.indicator` * event enrichment API logic * We were previously returning fields from `indicator.*`, we now include the `indicator.*` suffix in order to be more consistent with the sibling `matched.*` fields * row renderer logic * removal of dataset * updates relevant to API changes above * Fix logical error in generating links from indicator fields We want to link the reference field, not a `first_seen` field. * Always include the indicator prefix in first-party indicator fields Prior to this change we would display e.g. `threatintel.indicator.foo` for investigation enrichment fields. Now that the structure has changed slightly and we return both `indicator.*` and `matched.*` fields for existing enrichents, we want to display investigation enrichment similarly. * Update indicator match rule integration tests Now that we've updated our enrichment logic, we need to update our enrichment tests. * Remove unused translation * Update example row renderer data for enriched alerts * Update parallel CTI constants to get our CTI row renderer working We were not requesting the necessary fields for our row renderer, since these constants (specifically CTI_ROW_RENDERER_FIELDS) now exist in both security_solution and the timelines plugin. I had updated the former, but only the latter is actually used. * Update CTI enrichment UI tests * Update prepackaged threat timeline template with new threat fields Also bumps the timelineTemplateVersion. * Update Indicator Match rule tests These needed three things: * Update to timeline template (see previous commit) * Changing expectations from `threat.indicator` to `threat.enrichments` * Update row renderer expectation to exclude dataset * Update mock data with newest CTI enrichment fields * Fix assertion on our threat details These fields are prefixed with `indicator` now because: 1. This data pertains to the indicator, not the match per se 2. The actual field is prefixed with indicator (or, it at least specifies an indicator in the case of a custom threat index (via threat_indicator_path)) * Update test data and tests for our field parsing helpers * Update more event-parsing tests Ths one involved updating a mock in another package. * Modify our helper function to support old filebeat indicators When we query indicators for enrichment matches, the current expectation is that we'll be querying 7.14 filebeat modules, which have an indicator path of 'threatintel.indicator'. The only place that matters on the UI is on the threat intel panel, where these indicators come back with such a prefix. This change has one behavior: it brings back the `provider` field on the Alert summary tab for queried enrichments from filebeat modules. * Update variable and method names to be more consistent with internal terminology Indicators come from a CTI index. Enrichments are the application of indicator data to other documents, and contain both indicator fields and matched context. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Ryland Herrick <[email protected]>
Summary
This PR Is split into two logical chunks, committed sequentially:
Data Updates (diff)
threat.enrichments
threat.enrichments
mappingsthreat.enrichments
thanks to the index alias from RAC indices to older signals indicesthreat.enrichments
moving forward, and so this will enable backwards compatibility.App Updates (diff)
This involved updating application code to read/write from
threat.enrichments
instead ofthreat.indicator
. This touched the following areas:dataset
field from event renderer as that is not part of ECS nor deemed relevant indicator infothreat.enrichments
), legacy indicators (threatintel.indicator
), and future indicators (threat.indicator
)threat.enrichments
), legacy indicators (threatintel.indicator
), and future indicators (threat.indicator
)indicator
ormatched
, corresponding to ECSthreat.enrichments
threat.enrichments.matched
instead ofthreat.indicator.matched
Screenshots
new row renderer (sans dataset field)
Existing enrichments in Threat Intel tab
### 7.14 filebeat indicators in Threat intel tabReview steps
7.15
branch; create an indicator match rule and generate alertsthreat.enrichments
alerts in the existing indexthreat.enrichments
alerts will now have the proper mappingsChecklist
For maintainers