[Security Solution][CTI] Investigation time enrichment UI #103383
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's not being invoked yet, but I've added a placeholder where it's going.
This is a rough copy/paste, I'll clean up as I flesh out the new tests.
* Declares an enum for our types * Sets type during indicator match rule enrichment * Sets type during investigation-time enrichment
There are lots of TODOs here, but this implements the following: * Fetching investigation-time enrichments from the backend * Parsing existing enrichments from timeline data * Merging the two enrichment types together, and rendering them in rows as specified Much of the data-fetching is hardcoded, and this broke the existing pattern with SummaryView/SummaryRow so that got a little messy; I may end up just using my own EuiTable but we'll see. Threat Intel tab is currently broken; that's up next.
The investigation-time enrichments are a little messy because they contain all the non-ECS fields that indicators contain; other than that, this is looking good. Still need to add the new header, and potentially sort the fields.
This promotes sanity for the user.
This simply opens the threat intel tab.
This also addresses a bug where we were not properly sorting new enrichments by first_seen; this is covered under the tests that were fixed.
Because the enrichment endpoint is dumb and doesn't know about the existing event or its enrichments, we need to merge these together on the client to reduce noise and redundant data.
* Massages the response into the format that the inspect component uses * Moves stateful fetching of query and persisting in redux to new, more specialized hook * Moves existing enrichment hook to a more suitable location in containers/
rylnd
added
Team: SecuritySolution
* indicator match rule now specifies `matched.type` as coming from the rule * Inspecting the enrichment query requires use of the redux store, which was not previously mocked
This covers the basics of the Alert Summary and Threat Intel tabs; the investigation-time enrichment functionality is up next.
* Loads more indicators (filebeat data, `threat_indicator2` archive) AFTER the rule has executed * Asserts that those indicators are also found on the alert summary.
This was previously hardcoded during development.
The existing myhash field will generate an alert due to the way the rule is written, but the alert had no other fields that would match the investigation time enrichment. This gives it a source.ip, and updates the indicator to match.
If none of the alert's fields would be relevant to the enrichment query, then we don't make the request at all.
This field was updated to reflect the source of the match, in this case: indicator match rules.
ecezalp
reviewed
Jun 28, 2021
x-pack/plugins/security_solution/common/search_strategy/security_solution/cti/index.ts
Outdated
Show resolved
Hide resolved
...ins/security_solution/public/common/components/event_details/cti_details/enrichment_icon.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/event_details/cti_details/helpers.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/common/components/event_details/cti_details/helpers.tsx
Outdated
Show resolved
Hide resolved
| getEnrichmentValue(enrichment, `${DEFAULT_INDICATOR_SOURCE_PATH}.${field}`); | ||
|
|
||
| const buildEnrichmentId = (enrichment: CtiEnrichment): string => { | ||
| const matchedId = getEnrichmentValue(enrichment, MATCHED_ID); |
There was a problem hiding this comment.
do we need the shimmed values in here as well?
There was a problem hiding this comment.
Good question! Nope, these will always just be matched.*:
- existing enrichments come from parsing the value of
threat.indicator, thus implicitly removingthreat.indicatorfrom their field names - investigation enrichments use the new ECS format where
matched.*is a top-level field within a giventhreat.enrichmentsobject (which is what the API returns)
...security_solution/public/common/components/event_details/cti_details/threat_summary_view.tsx
Outdated
Show resolved
Hide resolved
| * | ||
| * @param enrichments {@type CtiEnrichment[]} | ||
| */ | ||
| export const filterDuplicateEnrichments = (enrichments: CtiEnrichment[]): CtiEnrichment[] => { |
There was a problem hiding this comment.
here it looks like we are iterating over all enrichments at all times, although my understanding is that we already have two types of enrichments that we could pass in as separate arguments, and we could check if either is empty, and if not we could check if the investigation time ones are within the rule-execution time ones only
There was a problem hiding this comment.
Your logic is correct, but this call is behind a useMemo on the enrichments themselves, and we do not expect there to be many enrichments matching a given event. Maintaining separate "buckets" for each enrichment type seems nice with two, but that may not scale either.
If this code ends up being an issue we can decide how to optimize it based on the demonstrated problem.
...urity_solution/public/common/containers/cti/event_enrichment/use_investigation_enrichment.ts
Outdated
Show resolved
Hide resolved
...urity_solution/public/common/containers/cti/event_enrichment/use_investigation_enrichment.ts
Outdated
Show resolved
Hide resolved
| field: query.field, | ||
| id: query.id, | ||
| index: query.index, | ||
| type: ENRICHMENT_TYPES.IndicatorMatchRule, |
There was a problem hiding this comment.
I see the answer to one of my previous questions. this works but it adds a bit of cognitive overhead IMHO, it could be better to use a new separate field to carry this information, such as indicatorType
There was a problem hiding this comment.
You're right that this is a little confusing, but matched.type was previously a useless field, wholly redundant with indicator.type. ECS has been updated to reflect this new use, and we can sweep these inconsistencies under the rug with the data migration in 7.15.
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Displays a loading spinner in the Threat Intel tab title, and some loading lines where the enrichments summary is.
This fixes our cypress test, but it's going to start failing again in 30 days. However, by that time I'll have implemented the absolute data picker, which will allow for a more comprehensive test in addition to us sidestepping this issue.
The name prop on a Tab will be rendered as a node, so both strings and elements are acceptable. This relaxes the types to inherit from the component itself.
The addition of our filtering of the search observable broke this test, since we now need to implement the search observable. Rather than do that, we'll instead mock our local hook as that's more likely to change.
Conflicts: x-pack/plugins/security_solution/cypress/integration/detection_rules/indicator_match_rule.spec.ts
rylnd
added
the
auto-backport
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
This override test is failing due to a mapping conflict, which causes the rule name override field to be invalid and not persisted. The mapping conflict is due to a typo in the threat_indicator2 archive, where the document declares one index but the mappings declare another. While es_archive happily loads the document into the index it specifies, the mappings already exist via the threat_indicator archive, and thus the new threat_indicator2 index receives dynamic mappings. Additionally, because the index created by threat_indicator2 is not the index specified in threat_indicator2's mappings, the behavior seems to be to leave that index when calling unload! Thus, on the next test that cares (override.spec.ts), we have an errant filebeat (threat_indicator2) index with default mappings, causing mapping conflicts with our other ECS indexes.
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @rylnd |
💔 Backport failed
To backport manually run: |
rylnd
added a commit
to rylnd/kibana
that referenced
this pull request
Jun 30, 2021
…3383) * Add pure fn and consuming hook to fetch event enrichment It's not being invoked yet, but I've added a placeholder where it's going. * Move existing enrichment tests to new spec file This is a rough copy/paste, I'll clean up as I flesh out the new tests. * Move test constants into tests that use them * style: declare FC function as an FC * Extract some inline parsing logic into a helper function And test it! * Solidifying enrichment types on the backend * Declares an enum for our types * Sets type during indicator match rule enrichment * Sets type during investigation-time enrichment * WIP: Enrichment rows are rendered on the alerts summary There are lots of TODOs here, but this implements the following: * Fetching investigation-time enrichments from the backend * Parsing existing enrichments from timeline data * Merging the two enrichment types together, and rendering them in rows as specified Much of the data-fetching is hardcoded, and this broke the existing pattern with SummaryView/SummaryRow so that got a little messy; I may end up just using my own EuiTable but we'll see. Threat Intel tab is currently broken; that's up next. * Updates ThreatDetailsView to accept an array of enrichments The investigation-time enrichments are a little messy because they contain all the non-ECS fields that indicators contain; other than that, this is looking good. Still need to add the new header, and potentially sort the fields. * Sort our details fields This promotes sanity for the user. * Add "view threat intel data" button This simply opens the threat intel tab. * Implement header for threat details sections * Add a basic jest "unit" test around ThreatSummaryView * Fix remaining tests for components we modified This also addresses a bug where we were not properly sorting new enrichments by first_seen; this is covered under the tests that were fixed. * Filter out duplicate investigation-time enrichments Because the enrichment endpoint is dumb and doesn't know about the existing event or its enrichments, we need to merge these together on the client to reduce noise and redundant data. * Add inspect button to investigation enrichments * Massages the response into the format that the inspect component uses * Moves stateful fetching of query and persisting in redux to new, more specialized hook * Moves existing enrichment hook to a more suitable location in containers/ * Fix failing unit tests * indicator match rule now specifies `matched.type` as coming from the rule * Inspecting the enrichment query requires use of the redux store, which was not previously mocked * Fix existing CTI cypress tests This covers the basics of the Alert Summary and Threat Intel tabs; the investigation-time enrichment functionality is up next. * Adds a cypress test exercising investigation time enrichment * Loads more indicators (filebeat data, `threat_indicator2` archive) AFTER the rule has executed * Asserts that those indicators are also found on the alert summary. * Populate event enrichment call with actual alert fields This was previously hardcoded during development. * Add a new field to our suspicious event to trigger enrichment The existing myhash field will generate an alert due to the way the rule is written, but the alert had no other fields that would match the investigation time enrichment. This gives it a source.ip, and updates the indicator to match. * Only fetch enrichments data if there are valid event fields If none of the alert's fields would be relevant to the enrichment query, then we don't make the request at all. * Update enrichments matched.typed in integration tests This field was updated to reflect the source of the match, in this case: indicator match rules. * Ensure draggable fields are unique in a multi-match scenario If a given field matched multiple indicators, then the previous contextId was not unique as it was based on field/value that matched. Adding provider to the mix would fix it, except that we're not guaranteed to have a provider. I've added both provider (if present) and an index value to the key to ensure that it's unique. * Simplify types This field can never be null, as we always set it in our response. * Move helper functioons out of shared location and into consuming component These are unlikely to be used elsewhere. * Clean up data parsing logic using reduce This obviates the need for our filter/guard function and the extra loop that it entails. We have to specify the return value of our reduce fn, however, but that's mostly equivalent to our type guard. * Move our general function into a general location * Extract the concept of "enrichment identifiers" This was already partially codified with 'buildEnrichmentId,' which is used to dedup enrichments; this extends the idea to all fields that could uniquely identify a given indicator. * Use existing constant as the source of our enrichments query This is now used by both the overview card and the enrichment query. * Codify our default enrichment lookback as constants * Remove unnecessary flexbox The generic SummaryView component previously had to deal with multi-valued CTI fields, representing the multiple values coming from the multiple nested objects with that field. However, with the new UI we no longer have that constraint, and so the default columnar style, and the corresponding overriding styles, are no longer necessary. * Filter out partial responses in the event enrichment observable The UI does not currently handle these. We need to test the behavior of long-running queries with this filter, but this should simplify the behavior to complete/error until we handle partial responses. * Display placeholders while event enrichment is loading Displays a loading spinner in the Threat Intel tab title, and some loading lines where the enrichments summary is. * Update our indicator data to be within the last 30 days This fixes our cypress test, but it's going to start failing again in 30 days. However, by that time I'll have implemented the absolute data picker, which will allow for a more comprehensive test in addition to us sidestepping this issue. * Fix type error with our details tabs The name prop on a Tab will be rendered as a node, so both strings and elements are acceptable. This relaxes the types to inherit from the component itself. * Fix failing jest tests The addition of our filtering of the search observable broke this test, since we now need to implement the search observable. Rather than do that, we'll instead mock our local hook as that's more likely to change. # Conflicts: # x-pack/plugins/security_solution/public/common/components/event_details/event_details.test.tsx
rylnd
added a commit
that referenced
this pull request
Jun 30, 2021
…103829) * Add pure fn and consuming hook to fetch event enrichment It's not being invoked yet, but I've added a placeholder where it's going. * Move existing enrichment tests to new spec file This is a rough copy/paste, I'll clean up as I flesh out the new tests. * Move test constants into tests that use them * style: declare FC function as an FC * Extract some inline parsing logic into a helper function And test it! * Solidifying enrichment types on the backend * Declares an enum for our types * Sets type during indicator match rule enrichment * Sets type during investigation-time enrichment * WIP: Enrichment rows are rendered on the alerts summary There are lots of TODOs here, but this implements the following: * Fetching investigation-time enrichments from the backend * Parsing existing enrichments from timeline data * Merging the two enrichment types together, and rendering them in rows as specified Much of the data-fetching is hardcoded, and this broke the existing pattern with SummaryView/SummaryRow so that got a little messy; I may end up just using my own EuiTable but we'll see. Threat Intel tab is currently broken; that's up next. * Updates ThreatDetailsView to accept an array of enrichments The investigation-time enrichments are a little messy because they contain all the non-ECS fields that indicators contain; other than that, this is looking good. Still need to add the new header, and potentially sort the fields. * Sort our details fields This promotes sanity for the user. * Add "view threat intel data" button This simply opens the threat intel tab. * Implement header for threat details sections * Add a basic jest "unit" test around ThreatSummaryView * Fix remaining tests for components we modified This also addresses a bug where we were not properly sorting new enrichments by first_seen; this is covered under the tests that were fixed. * Filter out duplicate investigation-time enrichments Because the enrichment endpoint is dumb and doesn't know about the existing event or its enrichments, we need to merge these together on the client to reduce noise and redundant data. * Add inspect button to investigation enrichments * Massages the response into the format that the inspect component uses * Moves stateful fetching of query and persisting in redux to new, more specialized hook * Moves existing enrichment hook to a more suitable location in containers/ * Fix failing unit tests * indicator match rule now specifies `matched.type` as coming from the rule * Inspecting the enrichment query requires use of the redux store, which was not previously mocked * Fix existing CTI cypress tests This covers the basics of the Alert Summary and Threat Intel tabs; the investigation-time enrichment functionality is up next. * Adds a cypress test exercising investigation time enrichment * Loads more indicators (filebeat data, `threat_indicator2` archive) AFTER the rule has executed * Asserts that those indicators are also found on the alert summary. * Populate event enrichment call with actual alert fields This was previously hardcoded during development. * Add a new field to our suspicious event to trigger enrichment The existing myhash field will generate an alert due to the way the rule is written, but the alert had no other fields that would match the investigation time enrichment. This gives it a source.ip, and updates the indicator to match. * Only fetch enrichments data if there are valid event fields If none of the alert's fields would be relevant to the enrichment query, then we don't make the request at all. * Update enrichments matched.typed in integration tests This field was updated to reflect the source of the match, in this case: indicator match rules. * Ensure draggable fields are unique in a multi-match scenario If a given field matched multiple indicators, then the previous contextId was not unique as it was based on field/value that matched. Adding provider to the mix would fix it, except that we're not guaranteed to have a provider. I've added both provider (if present) and an index value to the key to ensure that it's unique. * Simplify types This field can never be null, as we always set it in our response. * Move helper functioons out of shared location and into consuming component These are unlikely to be used elsewhere. * Clean up data parsing logic using reduce This obviates the need for our filter/guard function and the extra loop that it entails. We have to specify the return value of our reduce fn, however, but that's mostly equivalent to our type guard. * Move our general function into a general location * Extract the concept of "enrichment identifiers" This was already partially codified with 'buildEnrichmentId,' which is used to dedup enrichments; this extends the idea to all fields that could uniquely identify a given indicator. * Use existing constant as the source of our enrichments query This is now used by both the overview card and the enrichment query. * Codify our default enrichment lookback as constants * Remove unnecessary flexbox The generic SummaryView component previously had to deal with multi-valued CTI fields, representing the multiple values coming from the multiple nested objects with that field. However, with the new UI we no longer have that constraint, and so the default columnar style, and the corresponding overriding styles, are no longer necessary. * Filter out partial responses in the event enrichment observable The UI does not currently handle these. We need to test the behavior of long-running queries with this filter, but this should simplify the behavior to complete/error until we handle partial responses. * Display placeholders while event enrichment is loading Displays a loading spinner in the Threat Intel tab title, and some loading lines where the enrichments summary is. * Update our indicator data to be within the last 30 days This fixes our cypress test, but it's going to start failing again in 30 days. However, by that time I'll have implemented the absolute data picker, which will allow for a more comprehensive test in addition to us sidestepping this issue. * Fix type error with our details tabs The name prop on a Tab will be rendered as a node, so both strings and elements are acceptable. This relaxes the types to inherit from the component itself. * Fix failing jest tests The addition of our filtering of the search observable broke this test, since we now need to implement the search observable. Rather than do that, we'll instead mock our local hook as that's more likely to change. # Conflicts: # x-pack/plugins/security_solution/public/common/components/event_details/event_details.test.tsx
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Jun 30, 2021
…-png-pdf-report-type * 'master' of github.com:elastic/kibana: (178 commits) [test] Migrating to kbn_archiver from es_archiver - for the Maps app (elastic#103028) [Reporting] Reintroduce "ILM policy for managing reporting indices" (elastic#103850) [Security Solution][Endpoint] Allow activity log scrolling on small screens (elastic#103852) Allow zero (0) to unset unenroll_timeout field (elastic#103790) [TSVB] Metric count is depicted as `-` instead of 0 (elastic#103717) [Query] Es query/field base (elastic#103177) Remove add data button from nav (elastic#103810) Fix telemetry advanced setting style (elastic#103838) [Transform] Fix default naming and sorting fields suggestion for `top_metrics` agg (elastic#103690) [APM] use conventional error rate color for correlations (elastic#103500) Endpoint Telemetry: Agents Metrics + Policy Config / Response (elastic#102171) [Alerting] Fixed search results are not updated when search term is removed on Rules and Connectors page (elastic#103663) fix too many rernders (elastic#103672) [APM] Add “Analyze Data” button (elastic#103485) [Lens] Fix value popover spacing (elastic#103081) [TSVB] Fix TSVB is not reporting all categories of Elasticsearch error (elastic#102926) [SECURITY] Adds security links to doc link service (elastic#102676) Update dependency @elastic/charts to v31 (elastic#102078) [Security Solution][CTI] Investigation time enrichment UI (elastic#103383) Adds ECS guide to doc links service (elastic#102246) ... # Conflicts: # x-pack/plugins/reporting/public/share_context_menu/register_pdf_png_reporting.tsx
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This introduces the UI component to #101553. At a high level, this includes the following changes to the Event Details flyout:
Redesign of the Threat Summary section of the Alert Summary tab
Instead of listing curated CTI fields in a separate section, we now list the alert fields that matched indicators. The Threat Summary header was removed to reinforce that these are simply more fields from the alert, while the Security Solution shield logo was added to distinguish their meaning from the other fields.
Addition of contextual headers to the Threat Intel tab
To be more consistent/correlated with the information displayed on the Alert Summary tab, each Threat Intel section now includes a header including the same details as on the summary: which field matched (
matched.field), the value matched (matched.atomic), and the provider of the indicator.Inspection of event enrichment query
For investigation-time enrichments, the corresponding query is available for inspection on the Threat Intel tab:
TODO
Outstanding functionality
Checklist
For maintainers