From 447722e3d25c7f93867e3a3a583c6799c7d0f00c Mon Sep 17 00:00:00 2001 From: Marshall Main <55718608+marshallmain@users.noreply.github.com> Date: Fri, 20 Nov 2020 13:15:02 -0500 Subject: [PATCH] [Security Solution][Detections] Prevents recursive EQL rules (#82857) (#83932) * Prevents recursive EQL rules * Remove unused import Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../lib/detection_engine/signals/build_bulk_body.ts | 8 ++++++++ .../signals/signal_rule_alert_type.ts | 11 ++++++----- .../detection_engine/signals/single_bulk_create.ts | 6 +++--- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts index a704d076880b..e50956e9ef75 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts @@ -103,6 +103,14 @@ export const buildSignalGroupFromSequence = ( outputIndex ); + if ( + wrappedBuildingBlocks.some((block) => + block._source.signal?.ancestors.some((ancestor) => ancestor.rule === ruleSO.id) + ) + ) { + return []; + } + // Now that we have an array of building blocks for the events in the sequence, // we can build the signal that links the building blocks together // and also insert the group id (which is also the "shell" signal _id) in each building block diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index 4eda9150e52f..003626e31900 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -58,7 +58,7 @@ import { ruleStatusSavedObjectsClientFactory } from './rule_status_saved_objects import { getNotificationResultsLink } from '../notifications/utils'; import { TelemetryEventsSender } from '../../telemetry/sender'; import { buildEqlSearchRequest } from '../../../../common/detection_engine/get_query_filter'; -import { bulkInsertSignals } from './single_bulk_create'; +import { bulkInsertSignals, filterDuplicateSignals } from './single_bulk_create'; import { buildSignalFromEvent, buildSignalGroupFromSequence } from './build_bulk_body'; import { createThreatSignals } from './threat_mapping/create_threat_signals'; import { getIndexVersion } from '../routes/index/get_index_version'; @@ -495,16 +495,17 @@ export const signalRulesAlertType = ({ [] ); } else if (response.hits.events !== undefined) { - newSignals = response.hits.events.map((event) => - wrapSignal(buildSignalFromEvent(event, savedObject, true), outputIndex) + newSignals = filterDuplicateSignals( + savedObject.id, + response.hits.events.map((event) => + wrapSignal(buildSignalFromEvent(event, savedObject, true), outputIndex) + ) ); } else { throw new Error( 'eql query response should have either `sequences` or `events` but had neither' ); } - // TODO: replace with code that filters out recursive rule signals while allowing sequences and their building blocks - // const filteredSignals = filterDuplicateSignals(alertId, newSignals); if (newSignals.length > 0) { const insertResult = await bulkInsertSignals(newSignals, logger, services, refresh); result.bulkCreateTimes.push(insertResult.bulkCreateDuration); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_bulk_create.ts index d8889dcfcf47..8c1d4210a7b3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/single_bulk_create.ts @@ -7,7 +7,7 @@ import { countBy, isEmpty } from 'lodash'; import { performance } from 'perf_hooks'; import { AlertServices } from '../../../../../alerts/server'; -import { SignalSearchResponse, BulkResponse, SignalHit, BaseSignalHit } from './types'; +import { SignalSearchResponse, BulkResponse, BaseSignalHit } from './types'; import { RuleAlertAction } from '../../../../common/detection_engine/types'; import { RuleTypeParams, RefreshTypes } from '../types'; import { generateId, makeFloatString, errorAggregator } from './utils'; @@ -68,9 +68,9 @@ export const filterDuplicateRules = ( * @param ruleId The rule id * @param signals The candidate new signals */ -export const filterDuplicateSignals = (ruleId: string, signals: SignalHit[]) => { +export const filterDuplicateSignals = (ruleId: string, signals: BaseSignalHit[]) => { return signals.filter( - (doc) => !doc.signal.ancestors.some((ancestor) => ancestor.rule === ruleId) + (doc) => !doc._source.signal?.ancestors.some((ancestor) => ancestor.rule === ruleId) ); };