From d21e4f5af41369289023d2ead593b7a6d0bec436 Mon Sep 17 00:00:00 2001 From: Tiago Costa Date: Sat, 28 Nov 2020 03:49:04 +0000 Subject: [PATCH 1/2] skip flaky suite (#84440) --- x-pack/test/functional/apps/grok_debugger/grok_debugger.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/grok_debugger/grok_debugger.js b/x-pack/test/functional/apps/grok_debugger/grok_debugger.js index 771ca2088c055..8513429639e58 100644 --- a/x-pack/test/functional/apps/grok_debugger/grok_debugger.js +++ b/x-pack/test/functional/apps/grok_debugger/grok_debugger.js @@ -11,7 +11,8 @@ export default function ({ getService, getPageObjects }) { const PageObjects = getPageObjects(['grokDebugger']); - describe('grok debugger app', function () { + // FLAKY: https://github.com/elastic/kibana/issues/84440 + describe.skip('grok debugger app', function () { this.tags('includeFirefox'); before(async () => { await esArchiver.load('empty_kibana'); From bdf7b88b45329b29d8affcb5a0dc5452ed97210e Mon Sep 17 00:00:00 2001 From: Madison Caldwell Date: Sun, 29 Nov 2020 22:10:23 -0500 Subject: [PATCH 2/2] [Security Solution][Detections] Handle dupes when processing threshold rules (#83062) * Fix threshold rule synthetic signal generation * Use top_hits aggregation * Find signals and aggregate over search terms * Exclude dupes * Fixes to algorithm * Sync timestamps with events/signals * Add timestampOverride * Revert changes in signal creation * Simplify query, return 10k buckets * Account for when threshold.field is not supplied * Ensure we're getting the last event when threshold.field is not provided * Add missing import * Handle case where threshold field not supplied * Fix type errors * Handle non-ECS fields * Regorganize * Address comments * Fix type error * Add unit test for buildBulkBody on threshold results * Add threshold_count back to mapping (and deprecate) * Timestamp fixes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../routes/index/get_signals_template.ts | 2 +- .../routes/index/signals_mapping.json | 10 ++ .../signals/build_bulk_body.test.ts | 109 ++++++++++++++++++ .../signals/build_bulk_body.ts | 1 + .../detection_engine/signals/build_signal.ts | 4 +- .../signals/bulk_create_threshold_signals.ts | 10 +- .../find_previous_threshold_signals.ts | 87 ++++++++++++++ .../signals/find_threshold_signals.ts | 1 + .../signals/signal_rule_alert_type.ts | 45 +++++++- .../lib/detection_engine/signals/types.ts | 14 ++- .../endpoint/resolver/signals/mappings.json | 10 ++ .../es_archives/export_rule/mappings.json | 10 ++ 12 files changed, 296 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_previous_threshold_signals.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts index d1a9b701b2c9d..8ea1faa84cfba 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts @@ -7,7 +7,7 @@ import signalsMapping from './signals_mapping.json'; import ecsMapping from './ecs_mapping.json'; -export const SIGNALS_TEMPLATE_VERSION = 2; +export const SIGNALS_TEMPLATE_VERSION = 3; export const MIN_EQL_RULE_INDEX_VERSION = 2; export const getSignalsTemplate = (index: string) => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json index 4e9477f3f2f73..96868e62ea978 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json @@ -337,6 +337,16 @@ "threshold_count": { "type": "float" }, + "threshold_result": { + "properties": { + "count": { + "type": "long" + }, + "value": { + "type": "keyword" + } + } + }, "depth": { "type": "integer" } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts index ad060a9304e84..cd61fbcfd0fc7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts @@ -123,6 +123,115 @@ describe('buildBulkBody', () => { expect(fakeSignalSourceHit).toEqual(expected); }); + test('bulk body builds well-defined body with threshold results', () => { + const sampleParams = sampleRuleAlertParams(); + const baseDoc = sampleDocNoSortId(); + const doc: SignalSourceHit = { + ...baseDoc, + _source: { + ...baseDoc._source, + threshold_result: { + count: 5, + value: 'abcd', + }, + }, + }; + delete doc._source.source; + const fakeSignalSourceHit = buildBulkBody({ + doc, + ruleParams: sampleParams, + id: sampleRuleGuid, + name: 'rule-name', + actions: [], + createdAt: '2020-01-28T15:58:34.810Z', + updatedAt: '2020-01-28T15:59:14.004Z', + createdBy: 'elastic', + updatedBy: 'elastic', + interval: '5m', + enabled: true, + tags: ['some fake tag 1', 'some fake tag 2'], + throttle: 'no_actions', + }); + // Timestamp will potentially always be different so remove it for the test + // @ts-expect-error + delete fakeSignalSourceHit['@timestamp']; + const expected: Omit & { someKey: 'someValue' } = { + someKey: 'someValue', + event: { + kind: 'signal', + }, + signal: { + parent: { + id: sampleIdGuid, + type: 'event', + index: 'myFakeSignalIndex', + depth: 0, + }, + parents: [ + { + id: sampleIdGuid, + type: 'event', + index: 'myFakeSignalIndex', + depth: 0, + }, + ], + ancestors: [ + { + id: sampleIdGuid, + type: 'event', + index: 'myFakeSignalIndex', + depth: 0, + }, + ], + original_time: '2020-04-20T21:27:45+0000', + status: 'open', + rule: { + actions: [], + author: ['Elastic'], + building_block_type: 'default', + id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', + rule_id: 'rule-1', + false_positives: [], + max_signals: 10000, + risk_score: 50, + risk_score_mapping: [], + output_index: '.siem-signals', + description: 'Detecting root and admin users', + from: 'now-6m', + immutable: false, + index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'], + interval: '5m', + language: 'kuery', + license: 'Elastic License', + name: 'rule-name', + query: 'user.name: root or user.name: admin', + references: ['http://google.com'], + severity: 'high', + severity_mapping: [], + tags: ['some fake tag 1', 'some fake tag 2'], + threat: [], + throttle: 'no_actions', + type: 'query', + to: 'now', + note: '', + enabled: true, + created_by: 'elastic', + updated_by: 'elastic', + version: 1, + created_at: fakeSignalSourceHit.signal.rule?.created_at, + updated_at: fakeSignalSourceHit.signal.rule?.updated_at, + exceptions_list: getListArrayMock(), + }, + threshold_result: { + count: 5, + value: 'abcd', + }, + depth: 1, + }, + }; + expect(fakeSignalSourceHit).toEqual(expected); + }); + test('bulk body builds original_event if it exists on the event to begin with', () => { const sampleParams = sampleRuleAlertParams(); const doc = sampleDocNoSortId(); 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 e50956e9ef752..e6e6d2e0f5fa9 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 @@ -71,6 +71,7 @@ export const buildBulkBody = ({ ...buildSignal([doc], rule), ...additionalSignalFields(doc), }; + delete doc._source.threshold_result; const event = buildEventTypeSignal(doc); const signalHit: SignalHit = { ...doc._source, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts index b36a1cbb4a6b3..9cf2462526cfc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts @@ -96,9 +96,9 @@ export const buildSignal = (docs: BaseSignalHit[], rule: RulesSchema): Signal => export const additionalSignalFields = (doc: BaseSignalHit) => { return { parent: buildParent(removeClashes(doc)), - original_time: doc._source['@timestamp'], + original_time: doc._source['@timestamp'], // This field has already been replaced with timestampOverride, if provided. original_event: doc._source.event ?? undefined, - threshold_count: doc._source.threshold_count ?? undefined, + threshold_result: doc._source.threshold_result, original_signal: doc._source.signal != null && !isEventTypeSignal(doc) ? doc._source.signal : undefined, }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts index edaaa345d8a69..a98aae4ec8107 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts @@ -149,7 +149,10 @@ const getTransformedHits = ( const source = { '@timestamp': get(timestampOverride ?? '@timestamp', hit._source), - threshold_count: totalResults, + threshold_result: { + count: totalResults, + value: ruleId, + }, ...getThresholdSignalQueryFields(hit, filter), }; @@ -176,7 +179,10 @@ const getTransformedHits = ( const source = { '@timestamp': get(timestampOverride ?? '@timestamp', hit._source), - threshold_count: docCount, + threshold_result: { + count: docCount, + value: get(threshold.field, hit._source), + }, ...getThresholdSignalQueryFields(hit, filter), }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_previous_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_previous_threshold_signals.ts new file mode 100644 index 0000000000000..960693bc703d6 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_previous_threshold_signals.ts @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { TimestampOverrideOrUndefined } from '../../../../common/detection_engine/schemas/common/schemas'; +import { singleSearchAfter } from './single_search_after'; + +import { AlertServices } from '../../../../../alerts/server'; +import { Logger } from '../../../../../../../src/core/server'; +import { SignalSearchResponse } from './types'; +import { BuildRuleMessage } from './rule_messages'; + +interface FindPreviousThresholdSignalsParams { + from: string; + to: string; + indexPattern: string[]; + services: AlertServices; + logger: Logger; + ruleId: string; + bucketByField: string; + timestampOverride: TimestampOverrideOrUndefined; + buildRuleMessage: BuildRuleMessage; +} + +export const findPreviousThresholdSignals = async ({ + from, + to, + indexPattern, + services, + logger, + ruleId, + bucketByField, + timestampOverride, + buildRuleMessage, +}: FindPreviousThresholdSignalsParams): Promise<{ + searchResult: SignalSearchResponse; + searchDuration: string; + searchErrors: string[]; +}> => { + const aggregations = { + threshold: { + terms: { + field: 'signal.threshold_result.value', + }, + aggs: { + lastSignalTimestamp: { + max: { + field: 'signal.original_time', // timestamp of last event captured by bucket + }, + }, + }, + }, + }; + + const filter = { + bool: { + must: [ + { + term: { + 'signal.rule.rule_id': ruleId, + }, + }, + { + term: { + 'signal.rule.threshold.field': bucketByField, + }, + }, + ], + }, + }; + + return singleSearchAfter({ + aggregations, + searchAfterSortId: undefined, + timestampOverride, + index: indexPattern, + from, + to, + services, + logger, + filter, + pageSize: 0, + buildRuleMessage, + }); +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts index 01e4812b9c8bf..7141b61a23e6e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts @@ -51,6 +51,7 @@ export const findThresholdSignals = async ({ terms: { field: threshold.field, min_doc_count: threshold.value, + size: 10000, // max 10k buckets }, aggs: { // Get the most recent hit per bucket 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 003626e319007..f0b1825c7cc99 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 @@ -8,6 +8,7 @@ import { Logger, KibanaRequest } from 'src/core/server'; +import { Filter } from 'src/plugins/data/common'; import { SIGNALS_ID, DEFAULT_SEARCH_AFTER_PAGE_SIZE, @@ -29,6 +30,7 @@ import { RuleAlertAttributes, EqlSignalSearchResponse, BaseSignalHit, + ThresholdQueryBucket, } from './types'; import { getGapBetweenRuns, @@ -46,6 +48,7 @@ import { signalParamsSchema } from './signal_params_schema'; import { siemRuleActionGroups } from './siem_rule_action_groups'; import { findMlSignals } from './find_ml_signals'; import { findThresholdSignals } from './find_threshold_signals'; +import { findPreviousThresholdSignals } from './find_previous_threshold_signals'; import { bulkCreateMlSignals } from './bulk_create_ml_signals'; import { bulkCreateThresholdSignals } from './bulk_create_threshold_signals'; import { @@ -300,6 +303,46 @@ export const signalRulesAlertType = ({ lists: exceptionItems ?? [], }); + const { + searchResult: previousSignals, + searchErrors: previousSearchErrors, + } = await findPreviousThresholdSignals({ + indexPattern: [outputIndex], + from, + to, + services, + logger, + ruleId, + bucketByField: threshold.field, + timestampOverride, + buildRuleMessage, + }); + + previousSignals.aggregations.threshold.buckets.forEach((bucket: ThresholdQueryBucket) => { + esFilter.bool.filter.push(({ + bool: { + must_not: { + bool: { + must: [ + { + term: { + [threshold.field ?? 'signal.rule.rule_id']: bucket.key, + }, + }, + { + range: { + [timestampOverride ?? '@timestamp']: { + lte: bucket.lastSignalTimestamp.value_as_string, + }, + }, + }, + ], + }, + }, + }, + } as unknown) as Filter); + }); + const { searchResult: thresholdResults, searchErrors } = await findThresholdSignals({ inputIndexPattern: inputIndex, from, @@ -349,7 +392,7 @@ export const signalRulesAlertType = ({ }), createSearchAfterReturnType({ success, - errors: [...errors, ...searchErrors], + errors: [...errors, ...previousSearchErrors, ...searchErrors], createdSignalsCount: createdItemsCount, bulkCreateTimes: bulkCreateDuration ? [bulkCreateDuration] : [], }), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts index cda3c97c08531..9e81797b14731 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts @@ -46,6 +46,11 @@ export interface SignalsStatusParams { status: Status; } +export interface ThresholdResult { + count: number; + value: string; +} + export interface SignalSource { [key: string]: SearchTypes; // TODO: SignalSource is being used as the type for documents matching detection engine queries, but they may not @@ -67,6 +72,7 @@ export interface SignalSource { // signal.depth doesn't exist on pre-7.10 signals depth?: number; }; + threshold_result?: ThresholdResult; } export interface BulkItem { @@ -156,7 +162,7 @@ export interface Signal { original_time?: string; original_event?: SearchTypes; status: Status; - threshold_count?: SearchTypes; + threshold_result?: ThresholdResult; original_signal?: SearchTypes; depth: number; } @@ -239,3 +245,9 @@ export interface SearchAfterAndBulkCreateReturnType { export interface ThresholdAggregationBucket extends TermAggregationBucket { top_threshold_hits: BaseSearchResponse; } + +export interface ThresholdQueryBucket extends TermAggregationBucket { + lastSignalTimestamp: { + value_as_string: string; + }; +} diff --git a/x-pack/test/functional/es_archives/endpoint/resolver/signals/mappings.json b/x-pack/test/functional/es_archives/endpoint/resolver/signals/mappings.json index ad77961a41445..3c6042e8efd24 100644 --- a/x-pack/test/functional/es_archives/endpoint/resolver/signals/mappings.json +++ b/x-pack/test/functional/es_archives/endpoint/resolver/signals/mappings.json @@ -2582,6 +2582,16 @@ }, "threshold_count": { "type": "float" + }, + "threshold_result": { + "properties": { + "count": { + "type": "long" + }, + "value": { + "type": "keyword" + } + } } } }, diff --git a/x-pack/test/security_solution_cypress/es_archives/export_rule/mappings.json b/x-pack/test/security_solution_cypress/es_archives/export_rule/mappings.json index 757121df53d44..f701f811b244b 100644 --- a/x-pack/test/security_solution_cypress/es_archives/export_rule/mappings.json +++ b/x-pack/test/security_solution_cypress/es_archives/export_rule/mappings.json @@ -5131,6 +5131,16 @@ }, "threshold_count": { "type": "float" + }, + "threshold_result": { + "properties": { + "count": { + "type": "long" + }, + "value": { + "type": "keyword" + } + } } } },