From d312b6a2e4e863ab2ada0519ea2f65311dbc3a93 Mon Sep 17 00:00:00 2001 From: Marshall Main <55718608+marshallmain@users.noreply.github.com> Date: Tue, 12 Apr 2022 11:14:25 -0700 Subject: [PATCH] Use RuleDataReader to query for threshold signal history (#129763) (cherry picked from commit 4373d0aa81356a7e479de1ebd0b655cce41b51f3) # Conflicts: # x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.ts --- .../create_security_rule_type_wrapper.ts | 1 + .../threshold/create_threshold_alert_type.ts | 11 +- .../lib/detection_engine/rule_types/types.ts | 7 +- .../signals/executors/threshold.test.ts | 3 + .../signals/executors/threshold.ts | 9 +- .../get_threshold_signal_history.test.ts.snap | 115 ++++++++++++++++++ .../find_previous_threshold_signals.ts | 85 ------------- .../get_threshold_signal_history.test.ts | 32 +++++ .../threshold/get_threshold_signal_history.ts | 110 ++++++++++++----- 9 files changed, 253 insertions(+), 120 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/__snapshots__/get_threshold_signal_history.test.ts.snap delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.test.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts index f25bb16e90004..14bbc3a7fe0e8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_wrapper.ts @@ -261,6 +261,7 @@ export const createSecurityRuleTypeWrapper: CreateSecurityRuleTypeWrapper = tuple, wrapHits, wrapSequences, + ruleDataReader: ruleDataClient.getReader({ namespace: options.spaceId }), }, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/threshold/create_threshold_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/threshold/create_threshold_alert_type.ts index f7fb8a69fb089..e4994185fb504 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/threshold/create_threshold_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/threshold/create_threshold_alert_type.ts @@ -51,7 +51,15 @@ export const createThresholdAlertType = ( producer: SERVER_APP_ID, async executor(execOptions) { const { - runOpts: { buildRuleMessage, bulkCreate, exceptionItems, completeRule, tuple, wrapHits }, + runOpts: { + buildRuleMessage, + bulkCreate, + exceptionItems, + completeRule, + tuple, + wrapHits, + ruleDataReader, + }, services, startedAt, state, @@ -70,6 +78,7 @@ export const createThresholdAlertType = ( tuple, version, wrapHits, + ruleDataReader, }); return result; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts index 2537f7eeeaf72..8de33333dc9f8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts @@ -18,7 +18,11 @@ import { WithoutReservedActionGroups, } from '../../../../../alerting/common'; import { ListClient } from '../../../../../lists/server'; -import { PersistenceServices, IRuleDataClient } from '../../../../../rule_registry/server'; +import { + PersistenceServices, + IRuleDataClient, + IRuleDataReader, +} from '../../../../../rule_registry/server'; import { ConfigType } from '../../../config'; import { SetupPlugins } from '../../../plugin'; import { CompleteRule, RuleParams } from '../schemas/rule_schemas'; @@ -61,6 +65,7 @@ export interface RunOpts { }; wrapHits: WrapHits; wrapSequences: WrapSequences; + ruleDataReader: IRuleDataReader; } export type SecurityAlertType< diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.test.ts index df8492d803053..5256bb16eb28a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.test.ts @@ -18,6 +18,7 @@ import { buildRuleMessageFactory } from '../rule_messages'; import { sampleEmptyDocSearchResults } from '../__mocks__/es_results'; import { allowedExperimentalValues } from '../../../../../common/experimental_features'; import { ThresholdRuleParams } from '../../schemas/rule_schemas'; +import { createRuleDataClientMock } from '../../../../../../rule_registry/server/rule_data_client/rule_data_client.mock'; describe('threshold_executor', () => { const version = '8.0.0'; @@ -49,6 +50,7 @@ describe('threshold_executor', () => { describe('thresholdExecutor', () => { it('should set a warning when exception list for threshold rule contains value list exceptions', async () => { + const ruleDataClientMock = createRuleDataClientMock(); const exceptionItems = [getExceptionListItemSchemaMock({ entries: [getEntryListMock()] })]; const response = await thresholdExecutor({ completeRule: thresholdCompleteRule, @@ -69,6 +71,7 @@ describe('threshold_executor', () => { createdItems: [], })), wrapHits: jest.fn(), + ruleDataReader: ruleDataClientMock.getReader({ namespace: 'default' }), }); expect(response.warningMessages.length).toEqual(1); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.ts index 441baa7ee94fd..95505089420db 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/executors/threshold.ts @@ -41,6 +41,7 @@ import { BuildRuleMessage } from '../rule_messages'; import { ExperimentalFeatures } from '../../../../../common/experimental_features'; import { withSecuritySpan } from '../../../../utils/with_security_span'; import { buildThresholdSignalHistory } from '../threshold/build_signal_history'; +import { IRuleDataReader } from '../../../../../../rule_registry/server'; export const thresholdExecutor = async ({ completeRule, @@ -55,6 +56,7 @@ export const thresholdExecutor = async ({ state, bulkCreate, wrapHits, + ruleDataReader, }: { completeRule: CompleteRule; tuple: RuleRangeTuple; @@ -68,6 +70,7 @@ export const thresholdExecutor = async ({ state: ThresholdAlertState; bulkCreate: BulkCreate; wrapHits: WrapHits; + ruleDataReader: IRuleDataReader; }): Promise => { let result = createSearchAfterReturnType(); const ruleParams = completeRule.ruleParams; @@ -77,15 +80,11 @@ export const thresholdExecutor = async ({ const { signalHistory, searchErrors: previousSearchErrors } = state.initialized ? { signalHistory: state.signalHistory, searchErrors: [] } : await getThresholdSignalHistory({ - indexPattern: ['*'], // TODO: get outputIndex? from: tuple.from.toISOString(), to: tuple.to.toISOString(), - services, - logger, ruleId: ruleParams.ruleId, bucketByFields: ruleParams.threshold.field, - timestampOverride: ruleParams.timestampOverride, - buildRuleMessage, + ruleDataReader, }); if (!state.initialized) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/__snapshots__/get_threshold_signal_history.test.ts.snap b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/__snapshots__/get_threshold_signal_history.test.ts.snap new file mode 100644 index 0000000000000..de7413fe27926 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/__snapshots__/get_threshold_signal_history.test.ts.snap @@ -0,0 +1,115 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`buildPreviousThresholdAlertRequest should generate a proper request when bucketByFields contains multiple fields 1`] = ` +Object { + "body": Object { + "query": Object { + "bool": Object { + "must": Array [ + Object { + "range": Object { + "@timestamp": Object { + "format": "strict_date_optional_time", + "gte": "now-6m", + "lte": "now", + }, + }, + }, + Object { + "term": Object { + "signal.rule.rule_id": "threshold-rule", + }, + }, + Object { + "range": Object { + "signal.original_time": Object { + "gte": "now-6m", + }, + }, + }, + Object { + "bool": Object { + "minimum_should_match": 1, + "should": Array [ + Object { + "term": Object { + "signal.rule.threshold.field": "host.name", + }, + }, + Object { + "term": Object { + "kibana.alert.rule.parameters.threshold.field": "host.name", + }, + }, + ], + }, + }, + Object { + "bool": Object { + "minimum_should_match": 1, + "should": Array [ + Object { + "term": Object { + "signal.rule.threshold.field": "user.name", + }, + }, + Object { + "term": Object { + "kibana.alert.rule.parameters.threshold.field": "user.name", + }, + }, + ], + }, + }, + ], + }, + }, + "sort": Array [ + Object { + "@timestamp": "desc", + }, + ], + }, + "size": 10000, +} +`; + +exports[`buildPreviousThresholdAlertRequest should generate a proper request when bucketByFields is empty 1`] = ` +Object { + "body": Object { + "query": Object { + "bool": Object { + "must": Array [ + Object { + "range": Object { + "@timestamp": Object { + "format": "strict_date_optional_time", + "gte": "now-6m", + "lte": "now", + }, + }, + }, + Object { + "term": Object { + "signal.rule.rule_id": "threshold-rule", + }, + }, + Object { + "range": Object { + "signal.original_time": Object { + "gte": "now-6m", + }, + }, + }, + ], + }, + }, + "sort": Array [ + Object { + "@timestamp": "desc", + }, + ], + }, + "size": 10000, +} +`; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts deleted file mode 100644 index 1a2bfbf3a962d..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/find_previous_threshold_signals.ts +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { TimestampOverrideOrUndefined } from '../../../../../common/detection_engine/schemas/common/schemas'; -import { - AlertInstanceContext, - AlertInstanceState, - AlertServices, -} from '../../../../../../alerting/server'; -import { Logger } from '../../../../../../../../src/core/server'; -import { BuildRuleMessage } from '../rule_messages'; -import { singleSearchAfter } from '../single_search_after'; -import { SignalSearchResponse } from '../types'; - -interface FindPreviousThresholdSignalsParams { - from: string; - to: string; - indexPattern: string[]; - services: AlertServices; - logger: Logger; - ruleId: string; - bucketByFields: string[]; - timestampOverride: TimestampOverrideOrUndefined; - buildRuleMessage: BuildRuleMessage; -} - -export const findPreviousThresholdSignals = async ({ - from, - to, - indexPattern, - services, - logger, - ruleId, - bucketByFields, - timestampOverride, - buildRuleMessage, -}: FindPreviousThresholdSignalsParams): Promise<{ - searchResult: SignalSearchResponse; - searchDuration: string; - searchErrors: string[]; -}> => { - const filter = { - bool: { - must: [ - { - term: { - 'signal.rule.rule_id': ruleId, - }, - }, - // We might find a signal that was generated on the interval for old data... make sure to exclude those. - { - range: { - 'signal.original_time': { - gte: from, - }, - }, - }, - ...bucketByFields.map((field) => { - return { - term: { - 'signal.rule.threshold.field': field, - }, - }; - }), - ], - }, - }; - - return singleSearchAfter({ - searchAfterSortIds: undefined, - timestampOverride, - index: indexPattern, - from, - to, - services, - logger, - filter, - pageSize: 10000, // TODO: multiple pages? - buildRuleMessage, - }); -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.test.ts new file mode 100644 index 0000000000000..d9db2972a89d3 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.test.ts @@ -0,0 +1,32 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { buildPreviousThresholdAlertRequest } from './get_threshold_signal_history'; + +describe('buildPreviousThresholdAlertRequest', () => { + it('should generate a proper request when bucketByFields is empty', async () => { + const bucketByFields: string[] = []; + const to = 'now'; + const from = 'now-6m'; + const ruleId = 'threshold-rule'; + + expect( + buildPreviousThresholdAlertRequest({ from, to, ruleId, bucketByFields }) + ).toMatchSnapshot(); + }); + + it('should generate a proper request when bucketByFields contains multiple fields', async () => { + const bucketByFields: string[] = ['host.name', 'user.name']; + const to = 'now'; + const from = 'now-6m'; + const ruleId = 'threshold-rule'; + + expect( + buildPreviousThresholdAlertRequest({ from, to, ruleId, bucketByFields }) + ).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.ts index 276431c3bc929..c5fb69ef65471 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threshold/get_threshold_signal_history.ts @@ -5,60 +5,114 @@ * 2.0. */ -import { TimestampOverrideOrUndefined } from '../../../../../common/detection_engine/schemas/common/schemas'; -import { - AlertInstanceContext, - AlertInstanceState, - AlertServices, -} from '../../../../../../alerting/server'; -import { Logger } from '../../../../../../../../src/core/server'; +import * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { ThresholdSignalHistory } from '../types'; -import { BuildRuleMessage } from '../rule_messages'; -import { findPreviousThresholdSignals } from './find_previous_threshold_signals'; import { buildThresholdSignalHistory } from './build_signal_history'; +import { IRuleDataReader } from '../../../../../../rule_registry/server'; +import { createErrorsFromShard } from '../utils'; interface GetThresholdSignalHistoryParams { from: string; to: string; - indexPattern: string[]; - services: AlertServices; - logger: Logger; ruleId: string; bucketByFields: string[]; - timestampOverride: TimestampOverrideOrUndefined; - buildRuleMessage: BuildRuleMessage; + ruleDataReader: IRuleDataReader; } export const getThresholdSignalHistory = async ({ from, to, - indexPattern, - services, - logger, ruleId, bucketByFields, - timestampOverride, - buildRuleMessage, + ruleDataReader, }: GetThresholdSignalHistoryParams): Promise<{ signalHistory: ThresholdSignalHistory; searchErrors: string[]; }> => { - const { searchResult, searchErrors } = await findPreviousThresholdSignals({ - indexPattern, + const request = buildPreviousThresholdAlertRequest({ from, to, - services, - logger, ruleId, bucketByFields, - timestampOverride, - buildRuleMessage, }); + const response = await ruleDataReader.search(request); return { - signalHistory: buildThresholdSignalHistory({ - alerts: searchResult.hits.hits, + signalHistory: buildThresholdSignalHistory({ alerts: response.hits.hits }), + searchErrors: createErrorsFromShard({ + errors: response._shards.failures ?? [], }), - searchErrors, + }; +}; + +export const buildPreviousThresholdAlertRequest = ({ + from, + to, + ruleId, + bucketByFields, +}: { + from: string; + to: string; + ruleId: string; + bucketByFields: string[]; +}): estypes.SearchRequest => { + return { + size: 10000, + // We should switch over to @elastic/elasticsearch/lib/api/types instead of typesWithBodyKey where possible, + // but api/types doesn't have a complete type for `sort` + body: { + sort: [ + { + '@timestamp': 'desc', + }, + ], + query: { + bool: { + must: [ + { + range: { + '@timestamp': { + lte: to, + gte: from, + format: 'strict_date_optional_time', + }, + }, + }, + { + term: { + 'signal.rule.rule_id': ruleId, + }, + }, + // We might find a signal that was generated on the interval for old data... make sure to exclude those. + { + range: { + 'signal.original_time': { + gte: from, + }, + }, + }, + ...bucketByFields.map((field) => { + return { + bool: { + should: [ + { + term: { + 'signal.rule.threshold.field': field, + }, + }, + { + term: { + 'kibana.alert.rule.parameters.threshold.field': field, + }, + }, + ], + minimum_should_match: 1, + }, + }; + }), + ], + }, + }, + }, }; };