Skip to content

Commit 5d4162b

Browse files
[Security Solution][Detections] Handle dupes when processing threshold rules (#83062) (#84466)
* 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> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent e970045 commit 5d4162b

12 files changed

Lines changed: 296 additions & 7 deletions

File tree

x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import signalsMapping from './signals_mapping.json';
88
import ecsMapping from './ecs_mapping.json';
99

10-
export const SIGNALS_TEMPLATE_VERSION = 2;
10+
export const SIGNALS_TEMPLATE_VERSION = 3;
1111
export const MIN_EQL_RULE_INDEX_VERSION = 2;
1212

1313
export const getSignalsTemplate = (index: string) => {

x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/signals_mapping.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,16 @@
337337
"threshold_count": {
338338
"type": "float"
339339
},
340+
"threshold_result": {
341+
"properties": {
342+
"count": {
343+
"type": "long"
344+
},
345+
"value": {
346+
"type": "keyword"
347+
}
348+
}
349+
},
340350
"depth": {
341351
"type": "integer"
342352
}

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,115 @@ describe('buildBulkBody', () => {
123123
expect(fakeSignalSourceHit).toEqual(expected);
124124
});
125125

126+
test('bulk body builds well-defined body with threshold results', () => {
127+
const sampleParams = sampleRuleAlertParams();
128+
const baseDoc = sampleDocNoSortId();
129+
const doc: SignalSourceHit = {
130+
...baseDoc,
131+
_source: {
132+
...baseDoc._source,
133+
threshold_result: {
134+
count: 5,
135+
value: 'abcd',
136+
},
137+
},
138+
};
139+
delete doc._source.source;
140+
const fakeSignalSourceHit = buildBulkBody({
141+
doc,
142+
ruleParams: sampleParams,
143+
id: sampleRuleGuid,
144+
name: 'rule-name',
145+
actions: [],
146+
createdAt: '2020-01-28T15:58:34.810Z',
147+
updatedAt: '2020-01-28T15:59:14.004Z',
148+
createdBy: 'elastic',
149+
updatedBy: 'elastic',
150+
interval: '5m',
151+
enabled: true,
152+
tags: ['some fake tag 1', 'some fake tag 2'],
153+
throttle: 'no_actions',
154+
});
155+
// Timestamp will potentially always be different so remove it for the test
156+
// @ts-expect-error
157+
delete fakeSignalSourceHit['@timestamp'];
158+
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
159+
someKey: 'someValue',
160+
event: {
161+
kind: 'signal',
162+
},
163+
signal: {
164+
parent: {
165+
id: sampleIdGuid,
166+
type: 'event',
167+
index: 'myFakeSignalIndex',
168+
depth: 0,
169+
},
170+
parents: [
171+
{
172+
id: sampleIdGuid,
173+
type: 'event',
174+
index: 'myFakeSignalIndex',
175+
depth: 0,
176+
},
177+
],
178+
ancestors: [
179+
{
180+
id: sampleIdGuid,
181+
type: 'event',
182+
index: 'myFakeSignalIndex',
183+
depth: 0,
184+
},
185+
],
186+
original_time: '2020-04-20T21:27:45+0000',
187+
status: 'open',
188+
rule: {
189+
actions: [],
190+
author: ['Elastic'],
191+
building_block_type: 'default',
192+
id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
193+
rule_id: 'rule-1',
194+
false_positives: [],
195+
max_signals: 10000,
196+
risk_score: 50,
197+
risk_score_mapping: [],
198+
output_index: '.siem-signals',
199+
description: 'Detecting root and admin users',
200+
from: 'now-6m',
201+
immutable: false,
202+
index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'],
203+
interval: '5m',
204+
language: 'kuery',
205+
license: 'Elastic License',
206+
name: 'rule-name',
207+
query: 'user.name: root or user.name: admin',
208+
references: ['http://google.com'],
209+
severity: 'high',
210+
severity_mapping: [],
211+
tags: ['some fake tag 1', 'some fake tag 2'],
212+
threat: [],
213+
throttle: 'no_actions',
214+
type: 'query',
215+
to: 'now',
216+
note: '',
217+
enabled: true,
218+
created_by: 'elastic',
219+
updated_by: 'elastic',
220+
version: 1,
221+
created_at: fakeSignalSourceHit.signal.rule?.created_at,
222+
updated_at: fakeSignalSourceHit.signal.rule?.updated_at,
223+
exceptions_list: getListArrayMock(),
224+
},
225+
threshold_result: {
226+
count: 5,
227+
value: 'abcd',
228+
},
229+
depth: 1,
230+
},
231+
};
232+
expect(fakeSignalSourceHit).toEqual(expected);
233+
});
234+
126235
test('bulk body builds original_event if it exists on the event to begin with', () => {
127236
const sampleParams = sampleRuleAlertParams();
128237
const doc = sampleDocNoSortId();

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export const buildBulkBody = ({
7171
...buildSignal([doc], rule),
7272
...additionalSignalFields(doc),
7373
};
74+
delete doc._source.threshold_result;
7475
const event = buildEventTypeSignal(doc);
7576
const signalHit: SignalHit = {
7677
...doc._source,

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_signal.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ export const buildSignal = (docs: BaseSignalHit[], rule: RulesSchema): Signal =>
9696
export const additionalSignalFields = (doc: BaseSignalHit) => {
9797
return {
9898
parent: buildParent(removeClashes(doc)),
99-
original_time: doc._source['@timestamp'],
99+
original_time: doc._source['@timestamp'], // This field has already been replaced with timestampOverride, if provided.
100100
original_event: doc._source.event ?? undefined,
101-
threshold_count: doc._source.threshold_count ?? undefined,
101+
threshold_result: doc._source.threshold_result,
102102
original_signal:
103103
doc._source.signal != null && !isEventTypeSignal(doc) ? doc._source.signal : undefined,
104104
};

x-pack/plugins/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ const getTransformedHits = (
149149

150150
const source = {
151151
'@timestamp': get(timestampOverride ?? '@timestamp', hit._source),
152-
threshold_count: totalResults,
152+
threshold_result: {
153+
count: totalResults,
154+
value: ruleId,
155+
},
153156
...getThresholdSignalQueryFields(hit, filter),
154157
};
155158

@@ -176,7 +179,10 @@ const getTransformedHits = (
176179

177180
const source = {
178181
'@timestamp': get(timestampOverride ?? '@timestamp', hit._source),
179-
threshold_count: docCount,
182+
threshold_result: {
183+
count: docCount,
184+
value: get(threshold.field, hit._source),
185+
},
180186
...getThresholdSignalQueryFields(hit, filter),
181187
};
182188

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { TimestampOverrideOrUndefined } from '../../../../common/detection_engine/schemas/common/schemas';
8+
import { singleSearchAfter } from './single_search_after';
9+
10+
import { AlertServices } from '../../../../../alerts/server';
11+
import { Logger } from '../../../../../../../src/core/server';
12+
import { SignalSearchResponse } from './types';
13+
import { BuildRuleMessage } from './rule_messages';
14+
15+
interface FindPreviousThresholdSignalsParams {
16+
from: string;
17+
to: string;
18+
indexPattern: string[];
19+
services: AlertServices;
20+
logger: Logger;
21+
ruleId: string;
22+
bucketByField: string;
23+
timestampOverride: TimestampOverrideOrUndefined;
24+
buildRuleMessage: BuildRuleMessage;
25+
}
26+
27+
export const findPreviousThresholdSignals = async ({
28+
from,
29+
to,
30+
indexPattern,
31+
services,
32+
logger,
33+
ruleId,
34+
bucketByField,
35+
timestampOverride,
36+
buildRuleMessage,
37+
}: FindPreviousThresholdSignalsParams): Promise<{
38+
searchResult: SignalSearchResponse;
39+
searchDuration: string;
40+
searchErrors: string[];
41+
}> => {
42+
const aggregations = {
43+
threshold: {
44+
terms: {
45+
field: 'signal.threshold_result.value',
46+
},
47+
aggs: {
48+
lastSignalTimestamp: {
49+
max: {
50+
field: 'signal.original_time', // timestamp of last event captured by bucket
51+
},
52+
},
53+
},
54+
},
55+
};
56+
57+
const filter = {
58+
bool: {
59+
must: [
60+
{
61+
term: {
62+
'signal.rule.rule_id': ruleId,
63+
},
64+
},
65+
{
66+
term: {
67+
'signal.rule.threshold.field': bucketByField,
68+
},
69+
},
70+
],
71+
},
72+
};
73+
74+
return singleSearchAfter({
75+
aggregations,
76+
searchAfterSortId: undefined,
77+
timestampOverride,
78+
index: indexPattern,
79+
from,
80+
to,
81+
services,
82+
logger,
83+
filter,
84+
pageSize: 0,
85+
buildRuleMessage,
86+
});
87+
};

x-pack/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export const findThresholdSignals = async ({
5151
terms: {
5252
field: threshold.field,
5353
min_doc_count: threshold.value,
54+
size: 10000, // max 10k buckets
5455
},
5556
aggs: {
5657
// Get the most recent hit per bucket

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import { Logger, KibanaRequest } from 'src/core/server';
1010

11+
import { Filter } from 'src/plugins/data/common';
1112
import {
1213
SIGNALS_ID,
1314
DEFAULT_SEARCH_AFTER_PAGE_SIZE,
@@ -29,6 +30,7 @@ import {
2930
RuleAlertAttributes,
3031
EqlSignalSearchResponse,
3132
BaseSignalHit,
33+
ThresholdQueryBucket,
3234
} from './types';
3335
import {
3436
getGapBetweenRuns,
@@ -46,6 +48,7 @@ import { signalParamsSchema } from './signal_params_schema';
4648
import { siemRuleActionGroups } from './siem_rule_action_groups';
4749
import { findMlSignals } from './find_ml_signals';
4850
import { findThresholdSignals } from './find_threshold_signals';
51+
import { findPreviousThresholdSignals } from './find_previous_threshold_signals';
4952
import { bulkCreateMlSignals } from './bulk_create_ml_signals';
5053
import { bulkCreateThresholdSignals } from './bulk_create_threshold_signals';
5154
import {
@@ -300,6 +303,46 @@ export const signalRulesAlertType = ({
300303
lists: exceptionItems ?? [],
301304
});
302305

306+
const {
307+
searchResult: previousSignals,
308+
searchErrors: previousSearchErrors,
309+
} = await findPreviousThresholdSignals({
310+
indexPattern: [outputIndex],
311+
from,
312+
to,
313+
services,
314+
logger,
315+
ruleId,
316+
bucketByField: threshold.field,
317+
timestampOverride,
318+
buildRuleMessage,
319+
});
320+
321+
previousSignals.aggregations.threshold.buckets.forEach((bucket: ThresholdQueryBucket) => {
322+
esFilter.bool.filter.push(({
323+
bool: {
324+
must_not: {
325+
bool: {
326+
must: [
327+
{
328+
term: {
329+
[threshold.field ?? 'signal.rule.rule_id']: bucket.key,
330+
},
331+
},
332+
{
333+
range: {
334+
[timestampOverride ?? '@timestamp']: {
335+
lte: bucket.lastSignalTimestamp.value_as_string,
336+
},
337+
},
338+
},
339+
],
340+
},
341+
},
342+
},
343+
} as unknown) as Filter);
344+
});
345+
303346
const { searchResult: thresholdResults, searchErrors } = await findThresholdSignals({
304347
inputIndexPattern: inputIndex,
305348
from,
@@ -349,7 +392,7 @@ export const signalRulesAlertType = ({
349392
}),
350393
createSearchAfterReturnType({
351394
success,
352-
errors: [...errors, ...searchErrors],
395+
errors: [...errors, ...previousSearchErrors, ...searchErrors],
353396
createdSignalsCount: createdItemsCount,
354397
bulkCreateTimes: bulkCreateDuration ? [bulkCreateDuration] : [],
355398
}),

0 commit comments

Comments
 (0)