From f67fef9e50ea8f2756e50a40b30ea12e29df6cfe Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Wed, 24 Feb 2021 20:45:44 -0600 Subject: [PATCH] [Security Solution][Detections] Set default indicator path to reduce friction with new filebeat modules (#92081) * 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 <42973632+kibanamachine@users.noreply.github.com> --- .../security_solution/common/constants.ts | 8 +-- .../schemas/request/rule_schemas.mock.ts | 4 +- .../schemas/response/rules_schema.mocks.ts | 4 +- .../rules/step_about_rule/index.tsx | 4 +- .../rules/step_about_rule/schema.tsx | 2 +- .../threat_mapping/build_threat_enrichment.ts | 6 ++- .../enrich_signal_threat_matches.test.ts | 50 ++++++++++--------- .../enrich_signal_threat_matches.ts | 7 ++- .../tests/create_threat_matching.ts | 4 ++ 9 files changed, 51 insertions(+), 38 deletions(-) diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts index 31b4cef1a9d4..c277ce369dca 100644 --- 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/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts index 3bae9551f4df..a51c1f77844d 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts +++ b/x-pack/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/x-pack/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts index 8dc7427ed093..730e2949d7a1 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts +++ b/x-pack/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 = ({ euiFieldProps: { fullWidth: true, disabled: isLoading, - placeholder: DEFAULT_INDICATOR_PATH, + placeholder: DEFAULT_INDICATOR_SOURCE_PATH, }, }} /> diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx index 5e4d08c4e793..07012af17e73 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/schema.tsx @@ -205,7 +205,7 @@ export const schema: FormSchema = { '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/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts index cdbafe692c63..7d6cd655e336 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts +++ b/x-pack/plugins/security_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 => enrichSignalThreatMatches(signals, getMatchedThreats, defaultedIndicatorPath); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts index f98f0c88a2df..b77e8228e72d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts +++ b/x-pack/plugins/security_solution/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/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts index 761a58224fac..3c8b80886cab 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts +++ b/x-pack/plugins/security_solution/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/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_threat_matching.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_threat_matching.ts index 9e1c290d1605..20d2b107dc2c 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_threat_matching.ts +++ b/x-pack/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: [