-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution][Detections]Indicator Match Enrichment #89899
Changes from 32 commits
2d402b8
876b49d
9f9922d
9620fa8
2211110
e12c529
0b1dd8b
732a525
8fdf571
9712250
7936cb0
276f1c8
bb0584c
0d256c6
2da0633
e120609
fa1a8d2
aaec432
3e31d6a
537fa3c
12a0c87
5675a78
9481f81
fc03ba8
09cca36
e2e7552
679d138
e286aa6
52926af
ce60822
d526dfc
a96c773
a99978a
863e546
767739f
bd2a467
b6e176c
eca579b
9b8e343
0d8223f
ea0bbdd
1b973fc
bf35b55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
|
||
/* eslint-disable complexity */ | ||
|
||
import { identity } from 'lodash'; | ||
import { singleSearchAfter } from './single_search_after'; | ||
import { singleBulkCreate } from './single_bulk_create'; | ||
import { filterEventsAgainstList } from './filters/filter_events_against_list'; | ||
|
@@ -49,6 +50,7 @@ export const searchAfterAndBulkCreate = async ({ | |
tags, | ||
throttle, | ||
buildRuleMessage, | ||
enrichment = identity, | ||
}: SearchAfterAndBulkCreateParams): Promise<SearchAfterAndBulkCreateReturnType> => { | ||
let toReturn = createSearchAfterReturnType(); | ||
|
||
|
@@ -106,7 +108,7 @@ export const searchAfterAndBulkCreate = async ({ | |
services, | ||
logger, | ||
filter, | ||
pageSize: tuple.maxSignals < pageSize ? Math.ceil(tuple.maxSignals) : pageSize, // maximum number of docs to receive per search result. | ||
pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), | ||
rylnd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
timestampOverride: ruleParams.timestampOverride, | ||
excludeDocsWithTimestampOverride: true, | ||
}); | ||
|
@@ -117,14 +119,12 @@ export const searchAfterAndBulkCreate = async ({ | |
backupSortId = lastSortId[0]; | ||
hasBackupSortId = true; | ||
} else { | ||
// if no sort id on backup search and the initial search result was also empty | ||
logger.debug(buildRuleMessage('backupSortIds was empty on searchResultB')); | ||
hasBackupSortId = false; | ||
} | ||
|
||
mergedSearchResults = mergeSearchResults([mergedSearchResults, searchResultB]); | ||
|
||
// merge the search result from the secondary search with the first | ||
toReturn = mergeReturns([ | ||
toReturn, | ||
createSearchAfterReturnTypeFromResponse({ | ||
|
@@ -139,7 +139,6 @@ export const searchAfterAndBulkCreate = async ({ | |
} | ||
|
||
if (hasSortId) { | ||
// only execute search if we have something to sort on or if it is the first search | ||
const { searchResult, searchDuration, searchErrors } = await singleSearchAfter({ | ||
buildRuleMessage, | ||
searchAfterSortId: sortId, | ||
|
@@ -149,7 +148,7 @@ export const searchAfterAndBulkCreate = async ({ | |
services, | ||
logger, | ||
filter, | ||
pageSize: tuple.maxSignals < pageSize ? Math.ceil(tuple.maxSignals) : pageSize, // maximum number of docs to receive per search result. | ||
pageSize: Math.ceil(Math.min(tuple.maxSignals, pageSize)), | ||
timestampOverride: ruleParams.timestampOverride, | ||
excludeDocsWithTimestampOverride: false, | ||
}); | ||
|
@@ -166,6 +165,8 @@ export const searchAfterAndBulkCreate = async ({ | |
}), | ||
]); | ||
|
||
// TODO this comment was written when this code lived in a different | ||
// TODO location. We should verify whether this is still true | ||
rylnd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// we are guaranteed to have searchResult hits at this point | ||
// because we check before if the totalHits or | ||
// searchResult.hits.hits.length is 0 | ||
|
@@ -186,14 +187,6 @@ export const searchAfterAndBulkCreate = async ({ | |
buildRuleMessage(`searchResult.hit.hits.length: ${mergedSearchResults.hits.hits.length}`) | ||
); | ||
|
||
// search results yielded zero hits so exit | ||
// with search_after, these two values can be different when | ||
// searching with the last sortId of a consecutive search_after | ||
// yields zero hits, but there were hits using the previous | ||
// sortIds. | ||
// e.g. totalHits was 156, index 50 of 100 results, do another search-after | ||
// this time with a new sortId, index 22 of the remaining 56, get another sortId | ||
// search with that sortId, total is still 156 but the hits.hits array is empty. | ||
if (totalHits === 0 || mergedSearchResults.hits.hits.length === 0) { | ||
logger.debug( | ||
buildRuleMessage( | ||
|
@@ -228,6 +221,8 @@ export const searchAfterAndBulkCreate = async ({ | |
tuple.maxSignals - signalsCreatedCount | ||
); | ||
} | ||
const enrichedEvents = await enrichment(filteredEvents); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just observing here: @rylnd I think the big loop will exit early here if the enrichment function throws an error. Am I understanding that correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dhurley14 that's correct. The default function here won't throw, but for indicator match rules if they have e.g. a primitive value instead of nothing/an object, the signal creation would exit with an error. We could certainly try/catch this and continue to create signals without enrichments, but I'm not sure what the preferred behavior should be here. @FrankHassanabad @spong thoughts? I'll capture whatever behavior we decide here with an integration test. |
||
|
||
const { | ||
bulkCreateDuration: bulkDuration, | ||
createdItemsCount: createdCount, | ||
|
@@ -236,7 +231,7 @@ export const searchAfterAndBulkCreate = async ({ | |
errors: bulkErrors, | ||
} = await singleBulkCreate({ | ||
buildRuleMessage, | ||
filteredEvents, | ||
filteredEvents: enrichedEvents, | ||
ruleParams, | ||
services, | ||
logger, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These need to be updated with the latest from elastic/ecs#1127 before this gets merged/released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: as of today, these are up to date with the RFC.