[Security Solution][Detections] Set default indicator path to reduce …

…friction with new filebeat modules (#92081) (#92752) * Distinguish source and destination config for indicator matches We were previously conflating the path to retrieve indicator fields with the path to persist indicator fields, since they were the same value. To reduce friction in use with the new filebeat modules, we've decided to make the default source path threatintel.indicator. However, we still want to persist to threat.indicator, so we add a new constant, here. * Update our integration tests following change of default These tests were assuming a default path of threat.indicator. Since that is the ECS standard, we're not going to rewrite the tests but instead just add this rule override. In the future if the default changes, this parameter might be unnecessary. * DRY up unit tests a bit * Add a note for future devs If/when that constant changes, I imagine this will be useful context. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
elastic · Feb 25, 2021 · 827ee47 · 827ee47
1 parent 025ac2c
commit 827ee47
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 38 deletions.
diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts
@@ -45,10 +45,10 @@ export const DEFAULT_RULE_REFRESH_INTERVAL_VALUE = 60000; // ms
 export const DEFAULT_RULE_REFRESH_IDLE_VALUE = 2700000; // ms
 export const DEFAULT_RULE_NOTIFICATION_QUERY_SIZE = 100;
 
-// Document path where threat indicator fields are expected. Used as
-// both the source of enrichment fields and the destination for enrichment in
-// the generated detection alert
-export const DEFAULT_INDICATOR_PATH = 'threat.indicator';
+// Document path where threat indicator fields are expected. Fields are used
+// to enrich signals, and are copied to threat.indicator.
+export const DEFAULT_INDICATOR_SOURCE_PATH = 'threatintel.indicator';
+export const INDICATOR_DESTINATION_PATH = 'threat.indicator';
 
 export enum SecurityPageName {
   detections = 'detections',

diff --git a/...ck/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts b/...ck/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { DEFAULT_INDICATOR_PATH } from '../../../constants';
+import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../constants';
 import {
   MachineLearningCreateSchema,
   MachineLearningUpdateSchema,
@@ -57,7 +57,7 @@ export const getCreateThreatMatchRulesSchemaMock = (
   rule_id: ruleId,
   threat_query: '*:*',
   threat_index: ['list-index'],
-  threat_indicator_path: DEFAULT_INDICATOR_PATH,
+  threat_indicator_path: DEFAULT_INDICATOR_SOURCE_PATH,
   threat_mapping: [
     {
       entries: [

diff --git a/.../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts b/.../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { DEFAULT_INDICATOR_PATH } from '../../../constants';
+import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../constants';
 import { getListArrayMock } from '../types/lists.mock';
 
 import { RulesSchema } from './rules_schema';
@@ -151,7 +151,7 @@ export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<Rul
     language: 'kuery',
     threat_query: '*:*',
     threat_index: ['list-index'],
-    threat_indicator_path: DEFAULT_INDICATOR_PATH,
+    threat_indicator_path: DEFAULT_INDICATOR_SOURCE_PATH,
     threat_mapping: [
       {
         entries: [

diff --git a/...ck/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx b/...ck/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx
@@ -41,7 +41,7 @@ import { RiskScoreField } from '../risk_score_mapping';
 import { AutocompleteField } from '../autocomplete_field';
 import { useFetchIndex } from '../../../../common/containers/source';
 import { isThreatMatchRule } from '../../../../../common/detection_engine/utils';
-import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
+import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../../../common/constants';
 
 const CommonUseField = getUseField({ component: Field });
 
@@ -310,7 +310,7 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
                     euiFieldProps: {
                       fullWidth: true,
                       disabled: isLoading,
-                      placeholder: DEFAULT_INDICATOR_PATH,
+                      placeholder: DEFAULT_INDICATOR_SOURCE_PATH,
                     },
                   }}
                 />

diff --git a/...k/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx b/...k/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx
@@ -205,7 +205,7 @@ export const schema: FormSchema<AboutStepRule> = {
       'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.fieldThreatIndicatorPathHelpText',
       {
         defaultMessage:
-          'Specify the document path containing your threat indicator fields. Used for enrichment of indicator match alerts. Defaults to threat.indicator unless otherwise specified.',
+          'Specify the document path containing your threat indicator fields. Used for enrichment of indicator match alerts.',
       }
     ),
     labelAppend: OptionalFieldLabel,

diff --git a/...ty_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts b/...ty_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
+import { DEFAULT_INDICATOR_SOURCE_PATH } from '../../../../../common/constants';
 import { SignalSearchResponse, SignalsEnrichment } from '../types';
 import { enrichSignalThreatMatches } from './enrich_signal_threat_matches';
 import { getThreatList } from './get_threat_list';
@@ -52,7 +52,9 @@ export const buildThreatEnrichment = ({
     return threatResponse.hits.hits;
   };
 
-  const defaultedIndicatorPath = threatIndicatorPath ? threatIndicatorPath : DEFAULT_INDICATOR_PATH;
+  const defaultedIndicatorPath = threatIndicatorPath
+    ? threatIndicatorPath
+    : DEFAULT_INDICATOR_SOURCE_PATH;
   return (signals: SignalSearchResponse): Promise<SignalSearchResponse> =>
     enrichSignalThreatMatches(signals, getMatchedThreats, defaultedIndicatorPath);
 };
diff --git a/...n/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts b/...n/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts
@@ -6,7 +6,7 @@
  */
 
 import { get } from 'lodash';
-import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
+import { INDICATOR_DESTINATION_PATH } from '../../../../../common/constants';
 
 import { getThreatListItemMock } from './build_threat_mapping_filter.mock';
 import {
@@ -75,8 +75,10 @@ describe('groupAndMergeSignalMatches', () => {
 describe('buildMatchedIndicator', () => {
   let threats: ThreatListItem[];
   let queries: ThreatMatchNamedQuery[];
+  let indicatorPath: string;
 
   beforeEach(() => {
+    indicatorPath = 'threat.indicator';
     threats = [
       getThreatListItemMock({
         _id: '123',
@@ -94,7 +96,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries: [],
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toEqual([]);
@@ -104,7 +106,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(get(indicator, 'matched.atomic')).toEqual('domain_1');
@@ -114,7 +116,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(get(indicator, 'matched.field')).toEqual('event.field');
@@ -124,7 +126,7 @@ describe('buildMatchedIndicator', () => {
     const [indicator] = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(get(indicator, 'matched.type')).toEqual('type_1');
@@ -153,7 +155,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toHaveLength(queries.length);
@@ -163,7 +165,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toEqual([
@@ -228,7 +230,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toEqual([
@@ -253,7 +255,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toEqual([
@@ -285,7 +287,7 @@ describe('buildMatchedIndicator', () => {
     const indicators = buildMatchedIndicator({
       queries,
       threats,
-      indicatorPath: DEFAULT_INDICATOR_PATH,
+      indicatorPath,
     });
 
     expect(indicators).toEqual([
@@ -317,7 +319,7 @@ describe('buildMatchedIndicator', () => {
       buildMatchedIndicator({
         queries,
         threats,
-        indicatorPath: DEFAULT_INDICATOR_PATH,
+        indicatorPath,
       })
     ).toThrowError('Expected indicator field to be an object, but found: not an object');
   });
@@ -338,7 +340,7 @@ describe('buildMatchedIndicator', () => {
       buildMatchedIndicator({
         queries,
         threats,
-        indicatorPath: DEFAULT_INDICATOR_PATH,
+        indicatorPath,
       })
     ).toThrowError('Expected indicator field to be an object, but found: not an object');
   });
@@ -347,8 +349,10 @@ describe('buildMatchedIndicator', () => {
 describe('enrichSignalThreatMatches', () => {
   let getMatchedThreats: GetMatchedThreats;
   let matchedQuery: string;
+  let indicatorPath: string;
 
   beforeEach(() => {
+    indicatorPath = 'threat.indicator';
     getMatchedThreats = async () => [
       getThreatListItemMock({
         _id: '123',
@@ -367,7 +371,7 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      DEFAULT_INDICATOR_PATH
+      indicatorPath
     );
 
     expect(enrichedSignals.hits.hits).toEqual([]);
@@ -382,10 +386,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      DEFAULT_INDICATOR_PATH
+      indicatorPath
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
+    const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH);
 
     expect(indicators).toEqual([
       { existing: 'indicator' },
@@ -407,10 +411,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      DEFAULT_INDICATOR_PATH
+      indicatorPath
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
+    const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH);
 
     expect(indicators).toEqual([
       {
@@ -428,10 +432,10 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      DEFAULT_INDICATOR_PATH
+      indicatorPath
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
+    const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH);
 
     expect(indicators).toEqual([
       { existing: 'indicator' },
@@ -451,7 +455,7 @@ describe('enrichSignalThreatMatches', () => {
     });
     const signals = getSignalsResponseMock([signalHit]);
     await expect(() =>
-      enrichSignalThreatMatches(signals, getMatchedThreats, DEFAULT_INDICATOR_PATH)
+      enrichSignalThreatMatches(signals, getMatchedThreats, indicatorPath)
     ).rejects.toThrowError('Expected threat field to be an object, but found: whoops');
   });
 
@@ -487,7 +491,7 @@ describe('enrichSignalThreatMatches', () => {
       'custom_threat.custom_indicator'
     );
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
+    const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH);
 
     expect(indicators).toEqual([
       {
@@ -530,13 +534,13 @@ describe('enrichSignalThreatMatches', () => {
     const enrichedSignals = await enrichSignalThreatMatches(
       signals,
       getMatchedThreats,
-      DEFAULT_INDICATOR_PATH
+      indicatorPath
     );
     expect(enrichedSignals.hits.total).toEqual(expect.objectContaining({ value: 1 }));
     expect(enrichedSignals.hits.hits).toHaveLength(1);
 
     const [enrichedHit] = enrichedSignals.hits.hits;
-    const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);
+    const indicators = get(enrichedHit._source, INDICATOR_DESTINATION_PATH);
 
     expect(indicators).toEqual([
       {

diff --git a/...lution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts b/...lution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts
@@ -6,7 +6,6 @@
  */
 
 import { get, isObject } from 'lodash';
-import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
 
 import type { SignalSearchResponse, SignalSourceHit } from '../types';
 import type {
@@ -92,7 +91,11 @@ export const enrichSignalThreatMatches = async (
     if (!isObject(threat)) {
       throw new Error(`Expected threat field to be an object, but found: ${threat}`);
     }
-    const existingIndicatorValue = get(signalHit._source, DEFAULT_INDICATOR_PATH) ?? [];
+    // We are not using INDICATOR_DESTINATION_PATH here because the code above
+    // and below make assumptions about its current value, 'threat.indicator',
+    // and making this code dynamic on an arbitrary path would introduce several
+    // new issues.
+    const existingIndicatorValue = get(signalHit._source, 'threat.indicator') ?? [];
     const existingIndicators = [existingIndicatorValue].flat(); // ensure indicators is an array
 
     return {

diff --git a/...test/detection_engine_api_integration/security_and_spaces/tests/create_threat_matching.ts b/...test/detection_engine_api_integration/security_and_spaces/tests/create_threat_matching.ts
@@ -273,6 +273,7 @@ export default ({ getService }: FtrProviderContext) => {
             rule_id: 'rule-1',
             from: '1900-01-01T00:00:00.000Z',
             query: '*:*',
+            threat_indicator_path: 'threat.indicator',
             threat_query: 'threat.indicator.domain: *', // narrow things down to indicators with a domain
             threat_index: ['filebeat-*'], // Mimics indicators from the filebeat MISP module
             threat_mapping: [
@@ -353,6 +354,7 @@ export default ({ getService }: FtrProviderContext) => {
             rule_id: 'rule-1',
             from: '1900-01-01T00:00:00.000Z',
             query: 'source.port: 57324', // narrow our query to a single record that matches two indicators
+            threat_indicator_path: 'threat.indicator',
             threat_query: 'threat.indicator.ip: *',
             threat_index: ['filebeat-*'], // Mimics indicators from the filebeat MISP module
             threat_mapping: [
@@ -422,6 +424,7 @@ export default ({ getService }: FtrProviderContext) => {
             rule_id: 'rule-1',
             from: '1900-01-01T00:00:00.000Z',
             query: 'source.port: 57324', // narrow our query to a single record that matches two indicators
+            threat_indicator_path: 'threat.indicator',
             threat_query: 'threat.indicator.ip: *',
             threat_index: ['filebeat-*'], // Mimics indicators from the filebeat MISP module
             threat_mapping: [
@@ -519,6 +522,7 @@ export default ({ getService }: FtrProviderContext) => {
             rule_id: 'rule-1',
             from: '1900-01-01T00:00:00.000Z',
             query: '*:*', // narrow our query to a single record that matches two indicators
+            threat_indicator_path: 'threat.indicator',
             threat_query: '',
             threat_index: ['filebeat-*'], // Mimics indicators from the filebeat MISP module
             threat_mapping: [