From d44e04beb4817dd04803139e6fd4c1514f5b2435 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Wed, 7 Oct 2020 18:53:02 -0600 Subject: [PATCH] Adds date time query and return fields for timestamps and overrides (#79911) ## Summary Fixes https://github.com/elastic/kibana/issues/79865 Also fixes: * Timestamp override not being pushed down into threshold rules to use * Timestamp override not being used for lastValidDate * The return format of the date time might have been different depending on the customer mapping for both the override and the regular @timestamp so this fixes that as well. * Fixes one small type issue with fields. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../common/detection_engine/types.ts | 1 + .../signals/build_events_query.test.ts | 43 +++++++++ .../signals/build_events_query.ts | 23 +++++ .../detection_engine/signals/build_rule.ts | 4 +- .../signals/find_threshold_signals.ts | 9 +- .../signals/search_after_bulk_create.ts | 5 +- .../signals/signal_rule_alert_type.ts | 16 +++- .../detection_engine/signals/utils.test.ts | 94 ++++++++++++++++--- .../lib/detection_engine/signals/utils.ts | 45 ++++++--- .../security_solution/server/lib/types.ts | 1 - 10 files changed, 207 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/types.ts b/x-pack/plugins/security_solution/common/detection_engine/types.ts index bf57d7cb7815..6099a34f9afd 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/types.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/types.ts @@ -35,6 +35,7 @@ export interface BaseHit { _index: string; _id: string; _source: T; + fields?: Record; } export interface EqlSequence { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts index ccf8a9bec315..043066faa801 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts @@ -23,6 +23,12 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], query: { bool: { filter: [ @@ -37,6 +43,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -51,6 +58,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, @@ -94,6 +102,12 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], query: { bool: { filter: [ @@ -108,6 +122,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -122,6 +137,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, @@ -166,6 +182,12 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], query: { bool: { filter: [ @@ -180,6 +202,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -194,6 +217,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, @@ -239,6 +263,12 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], query: { bool: { filter: [ @@ -253,6 +283,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -267,6 +298,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, @@ -311,6 +343,12 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ], query: { bool: { filter: [ @@ -325,6 +363,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -339,6 +378,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, @@ -390,6 +430,7 @@ describe('create_signals', () => { size: 100, ignoreUnavailable: true, body: { + docvalue_fields: [{ field: '@timestamp', format: 'strict_date_optional_time' }], query: { bool: { filter: [ @@ -404,6 +445,7 @@ describe('create_signals', () => { range: { '@timestamp': { gte: 'now-5m', + format: 'strict_date_optional_time', }, }, }, @@ -418,6 +460,7 @@ describe('create_signals', () => { range: { '@timestamp': { lte: 'today', + format: 'strict_date_optional_time', }, }, }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts index 96db7e1eb53b..772645f06d76 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts @@ -28,6 +28,25 @@ export const buildEventsSearchQuery = ({ timestampOverride, }: BuildEventsSearchQuery) => { const timestamp = timestampOverride ?? '@timestamp'; + const docFields = + timestampOverride != null + ? [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + { + field: timestampOverride, + format: 'strict_date_optional_time', + }, + ] + : [ + { + field: '@timestamp', + format: 'strict_date_optional_time', + }, + ]; + const filterWithTime = [ filter, { @@ -40,6 +59,7 @@ export const buildEventsSearchQuery = ({ range: { [timestamp]: { gte: from, + format: 'strict_date_optional_time', }, }, }, @@ -54,6 +74,7 @@ export const buildEventsSearchQuery = ({ range: { [timestamp]: { lte: to, + format: 'strict_date_optional_time', }, }, }, @@ -65,12 +86,14 @@ export const buildEventsSearchQuery = ({ }, }, ]; + const searchQuery = { allowNoIndices: true, index, size, ignoreUnavailable: true, body: { + docvalue_fields: docFields, query: { bool: { filter: [ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_rule.ts index 6a76c7842e45..d7efc2d28699 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_rule.ts @@ -102,7 +102,7 @@ export const buildRule = ({ created_by: createdBy, updated_by: updatedBy, threat: ruleParams.threat ?? [], - timestamp_override: ruleParams.timestampOverride, // TODO: Timestamp Override via timestamp_override + timestamp_override: ruleParams.timestampOverride, throttle, version: ruleParams.version, created_at: createdAt, @@ -155,7 +155,7 @@ export const buildRuleWithoutOverrides = ( created_by: ruleSO.attributes.createdBy, updated_by: ruleSO.attributes.updatedBy, threat: ruleParams.threat ?? [], - timestamp_override: ruleParams.timestampOverride, // TODO: Timestamp Override via timestamp_override + timestamp_override: ruleParams.timestampOverride, throttle: ruleSO.attributes.throttle, version: ruleParams.version, created_at: ruleSO.attributes.createdAt, 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 282256804996..b34825b92ae9 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 @@ -6,7 +6,10 @@ import { isEmpty } from 'lodash/fp'; -import { Threshold } from '../../../../common/detection_engine/schemas/common/schemas'; +import { + Threshold, + TimestampOverrideOrUndefined, +} from '../../../../common/detection_engine/schemas/common/schemas'; import { singleSearchAfter } from './single_search_after'; import { AlertServices } from '../../../../../alerts/server'; @@ -23,6 +26,7 @@ interface FindThresholdSignalsParams { filter: unknown; threshold: Threshold; buildRuleMessage: BuildRuleMessage; + timestampOverride: TimestampOverrideOrUndefined; } export const findThresholdSignals = async ({ @@ -34,6 +38,7 @@ export const findThresholdSignals = async ({ filter, threshold, buildRuleMessage, + timestampOverride, }: FindThresholdSignalsParams): Promise<{ searchResult: SignalSearchResponse; searchDuration: string; @@ -54,7 +59,7 @@ export const findThresholdSignals = async ({ return singleSearchAfter({ aggregations, searchAfterSortId: undefined, - timestampOverride: undefined, + timestampOverride, index: inputIndexPattern, from, to, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts index 8fe55d97b569..0e6ddbc766fa 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.ts @@ -95,7 +95,10 @@ export const searchAfterAndBulkCreate = async ({ }); toReturn = mergeReturns([ toReturn, - createSearchAfterReturnTypeFromResponse({ searchResult }), + createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: ruleParams.timestampOverride, + }), createSearchAfterReturnType({ searchAfterTimes: [searchDuration], errors: searchErrors, 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 68fdbbeae1d8..838ac2558b03 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 @@ -99,6 +99,7 @@ export const signalRulesAlertType = ({ from, ruleId, index, + eventCategoryOverride, filters, language, maxSignals, @@ -114,6 +115,7 @@ export const signalRulesAlertType = ({ threatIndex, threatMapping, threatLanguage, + timestampOverride, type, exceptionsList, } = params; @@ -313,6 +315,7 @@ export const signalRulesAlertType = ({ logger, filter: esFilter, threshold, + timestampOverride, buildRuleMessage, }); @@ -346,7 +349,10 @@ export const signalRulesAlertType = ({ }); result = mergeReturns([ result, - createSearchAfterReturnTypeFromResponse({ searchResult: thresholdResults }), + createSearchAfterReturnTypeFromResponse({ + searchResult: thresholdResults, + timestampOverride, + }), createSearchAfterReturnType({ success, errors: [...errors, ...searchErrors], @@ -464,12 +470,12 @@ export const signalRulesAlertType = ({ const request = buildEqlSearchRequest( query, inputIndex, - params.from, - params.to, + from, + to, searchAfterSize, - params.timestampOverride, + timestampOverride, exceptionItems ?? [], - params.eventCategoryOverride + eventCategoryOverride ); const response: EqlSignalSearchResponse = await services.callCluster( 'transport.request', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts index 747b0bed3a04..157f741439bd 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.test.ts @@ -926,7 +926,10 @@ describe('utils', () => { describe('createSearchAfterReturnTypeFromResponse', () => { test('empty results will return successful type', () => { const searchResult = sampleEmptyDocSearchResults(); - const newSearchResult = createSearchAfterReturnTypeFromResponse({ searchResult }); + const newSearchResult = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); const expected: SearchAfterAndBulkCreateReturnType = { bulkCreateTimes: [], createdSignalsCount: 0, @@ -940,7 +943,10 @@ describe('utils', () => { test('multiple results will return successful type with expected success', () => { const searchResult = sampleDocSearchResultsWithSortId(); - const newSearchResult = createSearchAfterReturnTypeFromResponse({ searchResult }); + const newSearchResult = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); const expected: SearchAfterAndBulkCreateReturnType = { bulkCreateTimes: [], createdSignalsCount: 0, @@ -955,42 +961,60 @@ describe('utils', () => { test('result with error will create success: false within the result set', () => { const searchResult = sampleDocSearchResultsNoSortIdNoHits(); searchResult._shards.failed = 1; - const { success } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { success } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(success).toEqual(false); }); test('result with error will create success: false within the result set if failed is 2 or more', () => { const searchResult = sampleDocSearchResultsNoSortIdNoHits(); searchResult._shards.failed = 2; - const { success } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { success } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(success).toEqual(false); }); test('result with error will create success: true within the result set if failed is 0', () => { const searchResult = sampleDocSearchResultsNoSortIdNoHits(); searchResult._shards.failed = 0; - const { success } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { success } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(success).toEqual(true); }); test('It will not set an invalid date time stamp from a non-existent @timestamp when the index is not 100% ECS compliant', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = undefined; - const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(lastLookBackDate).toEqual(null); }); test('It will not set an invalid date time stamp from a null @timestamp when the index is not 100% ECS compliant', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = null; - const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(lastLookBackDate).toEqual(null); }); test('It will not set an invalid date time stamp from an invalid @timestamp string', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = 'invalid'; - const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ searchResult }); + const { lastLookBackDate } = createSearchAfterReturnTypeFromResponse({ + searchResult, + timestampOverride: undefined, + }); expect(lastLookBackDate).toEqual(null); }); }); @@ -999,30 +1023,76 @@ describe('utils', () => { test('It returns undefined if the search result contains a null timestamp', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = null; - const date = lastValidDate(searchResult); + const date = lastValidDate({ searchResult, timestampOverride: undefined }); expect(date).toEqual(undefined); }); test('It returns undefined if the search result contains a undefined timestamp', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = undefined; - const date = lastValidDate(searchResult); + const date = lastValidDate({ searchResult, timestampOverride: undefined }); expect(date).toEqual(undefined); }); test('It returns undefined if the search result contains an invalid string value', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = 'invalid value'; - const date = lastValidDate(searchResult); + const date = lastValidDate({ searchResult, timestampOverride: undefined }); expect(date).toEqual(undefined); }); test('It returns correct date time stamp if the search result contains an invalid string value', () => { const searchResult = sampleDocSearchResultsNoSortId(); (searchResult.hits.hits[0]._source['@timestamp'] as unknown) = 'invalid value'; - const date = lastValidDate(searchResult); + const date = lastValidDate({ searchResult, timestampOverride: undefined }); expect(date).toEqual(undefined); }); + + test('It returns normal date time if set', () => { + const searchResult = sampleDocSearchResultsNoSortId(); + const date = lastValidDate({ searchResult, timestampOverride: undefined }); + expect(date?.toISOString()).toEqual('2020-04-20T21:27:45.000Z'); + }); + + test('It returns date time from field if set there', () => { + const timestamp = '2020-10-07T19:27:19.136Z'; + const searchResult = sampleDocSearchResultsNoSortId(); + if (searchResult.hits.hits[0] == null) { + throw new TypeError('Test requires one element'); + } + searchResult.hits.hits[0] = { + ...searchResult.hits.hits[0], + fields: { + '@timestamp': [timestamp], + }, + }; + const date = lastValidDate({ searchResult, timestampOverride: undefined }); + expect(date?.toISOString()).toEqual(timestamp); + }); + + test('It returns timestampOverride date time if set', () => { + const override = '2020-10-07T19:20:28.049Z'; + const searchResult = sampleDocSearchResultsNoSortId(); + searchResult.hits.hits[0]._source.different_timestamp = new Date(override).toISOString(); + const date = lastValidDate({ searchResult, timestampOverride: 'different_timestamp' }); + expect(date?.toISOString()).toEqual(override); + }); + + test('It returns timestampOverride date time from fields if set on it', () => { + const override = '2020-10-07T19:36:31.110Z'; + const searchResult = sampleDocSearchResultsNoSortId(); + if (searchResult.hits.hits[0] == null) { + throw new TypeError('Test requires one element'); + } + searchResult.hits.hits[0] = { + ...searchResult.hits.hits[0], + fields: { + different_timestamp: [override], + }, + }; + const date = lastValidDate({ searchResult, timestampOverride: 'different_timestamp' }); + expect(date?.toISOString()).toEqual(override); + }); }); describe('createSearchAfterReturnType', () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts index 93e9622dcb6a..ac10f5ed9a72 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/utils.ts @@ -7,6 +7,7 @@ import { createHash } from 'crypto'; import moment from 'moment'; import dateMath from '@elastic/datemath'; +import { TimestampOverrideOrUndefined } from '../../../../common/detection_engine/schemas/common/schemas'; import { Logger, SavedObjectsClientContract } from '../../../../../../../src/core/server'; import { AlertServices, parseDuration } from '../../../../../alerts/server'; import { ExceptionListClient, ListClient, ListPluginSetup } from '../../../../../lists/server'; @@ -516,31 +517,53 @@ export const createErrorsFromShard = ({ errors }: { errors: ShardError[] }): str /** * Given a SignalSearchResponse this will return a valid last date if it can find one, otherwise it - * will return undefined. - * @param result The result to try and parse out the timestamp. + * will return undefined. This tries the "fields" first to get a formatted date time if it can, but if + * it cannot it will resort to using the "_source" fields second which can be problematic if the date time + * is not correctly ISO8601 or epoch milliseconds formatted. + * @param searchResult The result to try and parse out the timestamp. + * @param timestampOverride The timestamp override to use its values if we have it. */ -export const lastValidDate = (result: SignalSearchResponse): Date | undefined => { - if (result.hits.hits.length === 0) { +export const lastValidDate = ({ + searchResult, + timestampOverride, +}: { + searchResult: SignalSearchResponse; + timestampOverride: TimestampOverrideOrUndefined; +}): Date | undefined => { + if (searchResult.hits.hits.length === 0) { return undefined; } else { - const lastTimestamp = result.hits.hits[result.hits.hits.length - 1]._source['@timestamp']; - const isValid = lastTimestamp != null && moment(lastTimestamp).isValid(); - if (!isValid) { - return undefined; - } else { - return new Date(lastTimestamp); + const lastRecord = searchResult.hits.hits[searchResult.hits.hits.length - 1]; + const timestamp = timestampOverride ?? '@timestamp'; + const timestampValue = + lastRecord.fields != null && lastRecord.fields[timestamp] != null + ? lastRecord.fields[timestamp][0] + : lastRecord._source[timestamp]; + const lastTimestamp = + typeof timestampValue === 'string' || typeof timestampValue === 'number' + ? timestampValue + : undefined; + if (lastTimestamp != null) { + const tempMoment = moment(lastTimestamp); + if (tempMoment.isValid()) { + return tempMoment.toDate(); + } else { + return undefined; + } } } }; export const createSearchAfterReturnTypeFromResponse = ({ searchResult, + timestampOverride, }: { searchResult: SignalSearchResponse; + timestampOverride: TimestampOverrideOrUndefined; }): SearchAfterAndBulkCreateReturnType => { return createSearchAfterReturnType({ success: searchResult._shards.failed === 0, - lastLookBackDate: lastValidDate(searchResult), + lastLookBackDate: lastValidDate({ searchResult, timestampOverride }), }); }; diff --git a/x-pack/plugins/security_solution/server/lib/types.ts b/x-pack/plugins/security_solution/server/lib/types.ts index 36275d47c6a8..29db38bbbea6 100644 --- a/x-pack/plugins/security_solution/server/lib/types.ts +++ b/x-pack/plugins/security_solution/server/lib/types.ts @@ -78,7 +78,6 @@ export interface SearchResponse { _score: number; _version?: number; _explanation?: Explanation; - fields?: string[]; // eslint-disable-next-line @typescript-eslint/no-explicit-any highlight?: any; // eslint-disable-next-line @typescript-eslint/no-explicit-any