[Security Solution][Detection Engine] sets Indicator match rule sort order of search to asc (#176321)

## Summary

Sets search of documents for IM rule type from `desc` to `asc` when
suppression is enabled.
Also would allow to fix corner cases around [alert
suppression](https://github.com/elastic/kibana/pull/174241).
Alert suppression in IM rule relies on correct suppression time
boundaries to correctly deduplicate earlier suppressed alerts. I.e, if
document start suppression time(document timestamp) falls within
suppression boundaries, it means, alert was already suppressed. So, we
can exclude it from suppression as already suppressed and not to count
it twice.

But because documents for IM rule are searched in reverse order, it is
possible, while processing a second page of results, to falsely count
alert as already suppressed and discard it from suppressed count. That's
because its timestamp is older than document's timestamp from the first
page.
Newly added test failed only for code execution path, when number of
events is greater than number of threats.
It is because, events are split in chunks by 9,000 first. So if reverse
order in that case would cause alert from next batches to be dropped as
already suppressed

Setting `asc` can potentially affect IM rule performance, when events
need to be searched first and rule is configured with the large
look-back time. That's why new order is set to tech preview alert
suppression feature only

---------

Co-authored-by: Ryland Herrick <ryalnd@gmail.com>
This commit is contained in:
Vitalii Dmyterko 2024-02-08 12:21:30 +00:00 committed by GitHub
parent baa80de3d8
commit 59b986fbaa
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 166 additions and 37 deletions

View file

@ -5,8 +5,6 @@
* 2.0.
*/
import { firstValueFrom } from 'rxjs';
import { buildThreatMappingFilter } from './build_threat_mapping_filter';
import { getFilter } from '../../utils/get_filter';
import { searchAfterAndBulkCreate } from '../../utils/search_after_bulk_create';
@ -56,7 +54,8 @@ export const createEventSignal = async ({
inputIndexFields,
threatIndexFields,
completeRule,
licensing,
sortOrder = 'desc',
isAlertSuppressionActive,
}: CreateEventSignalOptions): Promise<SearchAfterAndBulkCreateReturnType> => {
const threatFiltersFromEvents = buildThreatMappingFilter({
threatMapping,
@ -65,9 +64,6 @@ export const createEventSignal = async ({
allowedFieldsForTermsQuery,
});
const license = await firstValueFrom(licensing.license$);
const hasPlatinumLicense = license.hasAtLeast('platinum');
if (!threatFiltersFromEvents.query || threatFiltersFromEvents.query?.bool.should.length === 0) {
// empty event list and we do not want to return everything as being
// a hit so opt to return the existing result.
@ -134,10 +130,6 @@ export const createEventSignal = async ({
threatSearchParams,
});
const isAlertSuppressionEnabled = Boolean(
completeRule.ruleParams.alertSuppression?.groupBy?.length
);
let createResult: SearchAfterAndBulkCreateReturnType;
const searchAfterBulkCreateParams = {
buildReasonMessage: buildReasonMessageForThreatMatchAlert,
@ -151,7 +143,7 @@ export const createEventSignal = async ({
pageSize: searchAfterSize,
ruleExecutionLogger,
services,
sortOrder: 'desc' as const,
sortOrder,
trackTotalHits: false,
tuple,
wrapHits,
@ -160,11 +152,7 @@ export const createEventSignal = async ({
secondaryTimestamp,
};
if (
isAlertSuppressionEnabled &&
runOpts.experimentalFeatures?.alertSuppressionForIndicatorMatchRuleEnabled &&
hasPlatinumLicense
) {
if (isAlertSuppressionActive) {
createResult = await searchAfterAndBulkCreateSuppressedAlerts({
...searchAfterBulkCreateParams,
wrapSuppressedHits,

View file

@ -5,8 +5,6 @@
* 2.0.
*/
import { firstValueFrom } from 'rxjs';
import { buildThreatMappingFilter } from './build_threat_mapping_filter';
import { getFilter } from '../../utils/get_filter';
import { searchAfterAndBulkCreate } from '../../utils/search_after_bulk_create';
@ -54,7 +52,8 @@ export const createThreatSignal = async ({
allowedFieldsForTermsQuery,
inputIndexFields,
threatIndexFields,
licensing,
sortOrder = 'desc',
isAlertSuppressionActive,
}: CreateThreatSignalOptions): Promise<SearchAfterAndBulkCreateReturnType> => {
const threatFilter = buildThreatMappingFilter({
threatMapping,
@ -63,9 +62,6 @@ export const createThreatSignal = async ({
allowedFieldsForTermsQuery,
});
const license = await firstValueFrom(licensing.license$);
const hasPlatinumLicense = license.hasAtLeast('platinum');
if (!threatFilter.query || threatFilter.query?.bool.should.length === 0) {
// empty threat list and we do not want to return everything as being
// a hit so opt to return the existing result.
@ -107,10 +103,6 @@ export const createThreatSignal = async ({
threatIndexFields,
});
const isAlertSuppressionEnabled = Boolean(
completeRule.ruleParams.alertSuppression?.groupBy?.length
);
let result: SearchAfterAndBulkCreateReturnType;
const searchAfterBulkCreateParams = {
buildReasonMessage: buildReasonMessageForThreatMatchAlert,
@ -124,7 +116,7 @@ export const createThreatSignal = async ({
pageSize: searchAfterSize,
ruleExecutionLogger,
services,
sortOrder: 'desc' as const,
sortOrder,
trackTotalHits: false,
tuple,
wrapHits,
@ -133,11 +125,7 @@ export const createThreatSignal = async ({
secondaryTimestamp,
};
if (
isAlertSuppressionEnabled &&
runOpts.experimentalFeatures?.alertSuppressionForIndicatorMatchRuleEnabled &&
hasPlatinumLicense
) {
if (isAlertSuppressionActive) {
result = await searchAfterAndBulkCreateSuppressedAlerts({
...searchAfterBulkCreateParams,
wrapSuppressedHits,

View file

@ -5,6 +5,8 @@
* 2.0.
*/
import { firstValueFrom } from 'rxjs';
import type { OpenPointInTimeResponse } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { uniq, chunk } from 'lodash/fp';
@ -216,6 +218,22 @@ export const createThreatSignals = async ({
}
};
const license = await firstValueFrom(licensing.license$);
const hasPlatinumLicense = license.hasAtLeast('platinum');
const isAlertSuppressionConfigured = Boolean(
completeRule.ruleParams.alertSuppression?.groupBy?.length
);
const isAlertSuppressionActive =
isAlertSuppressionConfigured &&
Boolean(runOpts.experimentalFeatures?.alertSuppressionForIndicatorMatchRuleEnabled) &&
hasPlatinumLicense;
// alert suppression needs to be performed on results searched in ascending order, so alert's suppression boundaries would be set correctly
// at the same time, there are concerns on performance of IM rule when sorting is set to asc, as it may lead to longer rule runs, since it will
// first go through alerts that might ve been processed in earlier executions, when look back interval set to large values (it can't be larger than 24h)
const sortOrder = isAlertSuppressionConfigured ? 'asc' : 'desc';
if (eventCount < threatListCount) {
await createSignals({
totalDocumentCount: eventCount,
@ -236,6 +254,7 @@ export const createThreatSignals = async ({
exceptionFilter,
eventListConfig,
indexFields: inputIndexFields,
sortOrder,
}),
createSignal: (slicedChunk) =>
@ -278,7 +297,8 @@ export const createThreatSignals = async ({
inputIndexFields,
threatIndexFields,
runOpts,
licensing,
sortOrder,
isAlertSuppressionActive,
}),
});
} else {
@ -342,7 +362,8 @@ export const createThreatSignals = async ({
inputIndexFields,
threatIndexFields,
runOpts,
licensing,
sortOrder,
isAlertSuppressionActive,
}),
});
}

View file

@ -29,6 +29,7 @@ export const getEventList = async ({
exceptionFilter,
eventListConfig,
indexFields,
sortOrder = 'desc',
}: EventsOptions): Promise<estypes.SearchResponse<EventDoc>> => {
const calculatedPerPage = perPage ?? MAX_PER_PAGE;
if (calculatedPerPage > 10000) {
@ -59,7 +60,7 @@ export const getEventList = async ({
filter: queryFilter,
primaryTimestamp,
secondaryTimestamp,
sortOrder: 'desc',
sortOrder,
trackTotalHits: false,
runtimeMappings,
overrideBody: eventListConfig,

View file

@ -120,7 +120,8 @@ export interface CreateThreatSignalOptions {
inputIndexFields: DataViewFieldBase[];
threatIndexFields: DataViewFieldBase[];
runOpts: RunOpts<ThreatRuleParams>;
licensing: LicensingPluginSetup;
sortOrder?: SortOrderOrUndefined;
isAlertSuppressionActive: boolean;
}
export interface CreateEventSignalOptions {
@ -163,7 +164,8 @@ export interface CreateEventSignalOptions {
inputIndexFields: DataViewFieldBase[];
threatIndexFields: DataViewFieldBase[];
runOpts: RunOpts<ThreatRuleParams>;
licensing: LicensingPluginSetup;
sortOrder?: SortOrderOrUndefined;
isAlertSuppressionActive: boolean;
}
type EntryKey = 'field' | 'value';
@ -312,6 +314,7 @@ export interface EventsOptions {
exceptionFilter: Filter | undefined;
eventListConfig?: OverrideBodyQuery;
indexFields: DataViewFieldBase[];
sortOrder?: SortOrderOrUndefined;
}
export interface EventDoc {

View file

@ -1126,6 +1126,133 @@ export default ({ getService }: FtrProviderContext) => {
});
});
// large number of documents gets processed in batches of 9,000
// rule should correctly go through them and suppress
// that can be an issue when search results returning in desc order
// this test is added to verify suppression works fine for this cases
it('should suppress alerts on large number of documents, more than 9,000', async () => {
const id = uuidv4();
const firstTimestamp = '2020-10-28T05:45:00.000Z';
const secondTimestamp = '2020-10-28T06:10:00.000Z';
await eventsFiller({
id,
count: 10000 * eventsCount,
timestamp: [firstTimestamp, secondTimestamp],
});
await threatsFiller({ id, count: 10000 * threatsCount, timestamp: firstTimestamp });
await indexGeneratedSourceDocuments({
docsCount: 60000,
interval: [firstTimestamp, '2020-10-28T05:35:50.000Z'],
seed: (index, _, timestamp) => ({
id,
'@timestamp': timestamp,
host: {
name: `host-${index}`,
},
agent: { name: 'agent-a' },
}),
});
await indexGeneratedSourceDocuments({
docsCount: 60000,
interval: [secondTimestamp, '2020-10-28T06:20:50.000Z'],
seed: (index, _, timestamp) => ({
id,
'@timestamp': timestamp,
host: {
name: `host-${index}`,
},
agent: { name: 'agent-a' },
}),
});
await addThreatDocuments({
id,
timestamp: firstTimestamp,
fields: {
host: {
name: 'host-80',
},
},
count: 1,
});
await addThreatDocuments({
id,
timestamp: firstTimestamp,
fields: {
host: {
name: 'host-14000',
},
},
count: 1,
});
await addThreatDocuments({
id,
timestamp: firstTimestamp,
fields: {
host: {
name: 'host-36000',
},
},
count: 1,
});
await addThreatDocuments({
id,
timestamp: firstTimestamp,
fields: {
host: {
name: 'host-5700',
},
},
count: 1,
});
const rule: ThreatMatchRuleCreateProps = {
...indicatorMatchRule(id),
alert_suppression: {
group_by: ['agent.name'],
missing_fields_strategy: 'suppress',
duration: {
value: 300,
unit: 'm',
},
},
from: 'now-35m',
interval: '30m',
};
const { previewId } = await previewRule({
supertest,
rule,
timeframeEnd: new Date('2020-10-28T06:30:00.000Z'),
invocationCount: 2,
});
const previewAlerts = await getPreviewAlerts({
es,
previewId,
sort: ['agent.name', ALERT_ORIGINAL_TIME],
});
expect(previewAlerts.length).toEqual(1);
expect(previewAlerts[0]._source).toEqual({
...previewAlerts[0]._source,
[ALERT_SUPPRESSION_TERMS]: [
{
field: 'agent.name',
value: ['agent-a'],
},
],
// There 4 documents in threats index, each matches one document in source index on each of 2 rule executions
// In total it gives 8 potential alerts. With suppression enabled 1 is created, the rest 7 are suppressed
[ALERT_SUPPRESSION_DOCS_COUNT]: 7,
});
});
describe('rule execution only', () => {
it('should suppress alerts during rule execution only', async () => {
const id = uuidv4();
@ -2064,6 +2191,7 @@ export default ({ getService }: FtrProviderContext) => {
},
],
[ALERT_SUPPRESSION_DOCS_COUNT]: 499,
[ALERT_SUPPRESSION_START]: '2020-10-28T06:50:00.000Z',
});
});