Skip to content
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

Merged
merged 43 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2d402b8
Adds basic integration test for threat enrichment
rylnd Jan 31, 2021
876b49d
Update signals mappings with indicator fields
rylnd Jan 31, 2021
9f9922d
Simplify some ternaries with Math.min
rylnd Jan 21, 2021
9620fa8
Remove outdated comments
rylnd Jan 21, 2021
2211110
Add notes from walkthrough with devin
rylnd Jan 21, 2021
e12c529
Add an enrichment hook to the current signal creation pipeline
rylnd Feb 1, 2021
0b1dd8b
Add utility functions for encoding/decoding our threat query
rylnd Feb 1, 2021
732a525
Add a name to each threat match filter clause
rylnd Feb 1, 2021
8fdf571
Adds functions for signal enrichment of threat indicators
rylnd Feb 1, 2021
9712250
Wire up threat enrichment to threat match rules
rylnd Feb 1, 2021
7936cb0
Fleshes out threat match integration tests
rylnd Feb 1, 2021
276f1c8
Add more test cases to indicator match integration tests
rylnd Feb 1, 2021
bb0584c
Implement signal deduplification
rylnd Feb 3, 2021
0d256c6
Move default indicator path to constant
rylnd Feb 3, 2021
2da0633
Testing some edge cases with signal enrichment
rylnd Feb 3, 2021
e120609
Cover and test edge cases with threat enrichment generation
rylnd Feb 3, 2021
fa1a8d2
Fix logical error in TI enrichment
rylnd Feb 3, 2021
aaec432
Document behavior when an indicator matched but is absent on enrichment
rylnd Feb 3, 2021
3e31d6a
Add followup note
rylnd Feb 3, 2021
537fa3c
Add basic unit test for our enrichment function
rylnd Feb 3, 2021
12a0c87
Update license headers for new files
rylnd Feb 4, 2021
5675a78
Remove unused threatintel archive
rylnd Feb 5, 2021
9481f81
Bump signals version to allows some updates in patch releases
rylnd Feb 5, 2021
fc03ba8
Fix typings of threat list item
rylnd Feb 5, 2021
09cca36
Update test mock to be aware of (but not care about) named queries
rylnd Feb 5, 2021
e2e7552
Merge branch 'master' into threat_enrichment_simple
rylnd Feb 5, 2021
679d138
Remove/update outdated comments
rylnd Feb 5, 2021
e286aa6
Remove outdated comment
rylnd Feb 5, 2021
52926af
Update enriched signals' total to account for deduplication
rylnd Feb 5, 2021
ce60822
Remove development comments
rylnd Feb 5, 2021
d526dfc
Merge branch 'master' into threat_enrichment_simple
kibanamachine Feb 8, 2021
a96c773
Add JSDoc for our special template version constant
rylnd Feb 8, 2021
a99978a
Remove outdated comments
rylnd Feb 10, 2021
863e546
Add an additional test permutation for error cases
rylnd Feb 10, 2021
767739f
Remove unnecessary coalescing
rylnd Feb 10, 2021
bd2a467
Move logic to build threat enrichment function into helper
rylnd Feb 10, 2021
b6e176c
Refactor code to allow typescript to infer our type narrowing
rylnd Feb 10, 2021
eca579b
Use a POJO over a Map
rylnd Feb 10, 2021
9b8e343
Explicitly type our enriched signals
rylnd Feb 10, 2021
0d8223f
Add an explanatory note about these test results
rylnd Feb 10, 2021
ea0bbdd
Merge branch 'master' into threat_enrichment_simple
kibanamachine Feb 10, 2021
1b973fc
Remove unused imports
rylnd Feb 11, 2021
bf35b55
Remove threat mappings accidentally brought in with indicator work
rylnd Feb 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2457,6 +2457,144 @@
"ignore_above": 1024,
"type": "keyword"
},
"indicator": {
Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

"type": "nested",
"properties": {
"as": {
"properties": {
"number": {
"type": "long"
},
"organization": {
"properties": {
"name": {
"fields": {
"text": {
"norms": false,
"type": "text"
}
},
"ignore_above": 1024,
"type": "keyword"
}
}
}
}
},
"confidence": {
"ignore_above": 1024,
"type": "keyword"
},
"dataset": {
"ignore_above": 1024,
"type": "keyword"
},
"description": {
"type": "wildcard"
},
"domain": {
"ignore_above": 1024,
"type": "keyword"
},
"email": {
"properties": {
"address": {
"ignore_above": 1024,
"type": "keyword"
}
}
},
"first_seen": {
"type": "date"
},
"geo": {
"properties": {
"city_name": {
"ignore_above": 1024,
"type": "keyword"
},
"continent_name": {
"ignore_above": 1024,
"type": "keyword"
},
"country_iso_code": {
"ignore_above": 1024,
"type": "keyword"
},
"country_name": {
"ignore_above": 1024,
"type": "keyword"
},
"location": {
"type": "geo_point"
},
"name": {
"ignore_above": 1024,
"type": "keyword"
},
"region_iso_code": {
"ignore_above": 1024,
"type": "keyword"
},
"region_name": {
"ignore_above": 1024,
"type": "keyword"
}
}
},
"ip": {
"type": "ip"
},
"last_seen": {
"type": "date"
},
"marking": {
"properties": {
"tlp": {
"ignore_above": 1024,
"type": "keyword"
}
}
},
"matched": {
"properties": {
"atomic": {
"ignore_above": 1024,
"type": "keyword"
},
"field": {
"ignore_above": 1024,
"type": "keyword"
},
"type": {
"ignore_above": 1024,
"type": "keyword"
}
}
},
"module": {
"ignore_above": 1024,
"type": "keyword"
},
"port": {
"type": "long"
},
"provider": {
"ignore_above": 1024,
"type": "keyword"
},
"scanner_stats": {
"type": "long"
},
"sightings": {
"type": "long"
},
"type": {
"ignore_above": 1024,
"type": "keyword"
}
}
},
"tactic": {
"properties": {
"id": {
Expand Down Expand Up @@ -2492,6 +2630,28 @@
"reference": {
"ignore_above": 1024,
"type": "keyword"
},
"subtechnique": {
"properties": {
"id": {
"ignore_above": 1024,
"type": "keyword"
},
"name": {
"fields": {
"text": {
"norms": false,
"type": "text"
}
},
"ignore_above": 1024,
"type": "keyword"
},
"reference": {
"ignore_above": 1024,
"type": "keyword"
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import signalsMapping from './signals_mapping.json';
import ecsMapping from './ecs_mapping.json';

export const SIGNALS_TEMPLATE_VERSION = 14;
export const SIGNALS_TEMPLATE_VERSION = 15;
export const MIN_EQL_RULE_INDEX_VERSION = 2;

export const getSignalsTemplate = (index: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
repeatedSearchResultsWithSortId,
repeatedSearchResultsWithNoSortId,
sampleDocSearchResultsNoSortIdNoHits,
sampleDocWithSortId,
} from './__mocks__/es_results';
import { searchAfterAndBulkCreate } from './search_after_bulk_create';
import { buildRuleMessageFactory } from './rule_messages';
Expand Down Expand Up @@ -870,4 +871,93 @@ describe('searchAfterAndBulkCreate', () => {
expect(createdSignalsCount).toEqual(4);
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
});

it('invokes the enrichment callback with signal search results', async () => {
const sampleParams = sampleRuleAlertParams(30);
mockService.callCluster
.mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
create: {
status: 201,
},
},
],
})
.mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(3, 6)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
create: {
status: 201,
},
},
],
})
.mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(6, 9)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
create: {
status: 201,
},
},
],
})
.mockResolvedValueOnce(sampleDocSearchResultsNoSortIdNoHits());

const mockEnrichment = jest.fn((a) => a);
const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({
enrichment: mockEnrichment,
ruleParams: sampleParams,
gap: moment.duration(2, 'm'),
previousStartedAt: moment().subtract(10, 'm').toDate(),
listClient,
exceptionsList: [],
services: mockService,
logger: mockLogger,
eventsTelemetry: undefined,
id: sampleRuleGuid,
inputIndexPattern,
signalsIndex: DEFAULT_SIGNALS_INDEX,
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,
pageSize: 1,
filter: undefined,
refresh: false,
tags: ['some fake tag 1', 'some fake tag 2'],
throttle: 'no_actions',
buildRuleMessage,
});

expect(mockEnrichment).toHaveBeenCalledWith(
expect.objectContaining({
hits: expect.objectContaining({
hits: expect.arrayContaining([
expect.objectContaining({
...sampleDocWithSortId(),
_id: expect.any(String),
}),
]),
}),
})
);
expect(success).toEqual(true);
expect(mockService.callCluster).toHaveBeenCalledTimes(7);
expect(createdSignalsCount).toEqual(3);
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -49,6 +50,7 @@ export const searchAfterAndBulkCreate = async ({
tags,
throttle,
buildRuleMessage,
enrichment = identity,
}: SearchAfterAndBulkCreateParams): Promise<SearchAfterAndBulkCreateReturnType> => {
let toReturn = createSearchAfterReturnType();

Expand Down Expand Up @@ -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,
});
Expand All @@ -117,13 +119,16 @@ export const searchAfterAndBulkCreate = async ({
backupSortId = lastSortId[0];
hasBackupSortId = true;
} else {
// TODO: This comment does not seem to match the code; this is the first loop and the first search, so how could the initial search result be known?
// answer: these were previously not blocking searches, and so the "second" search actually executed first TLDR outdated comment
// 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]);

// TODO again, on the first loop this is the FIRST search and toReturn is default values
// merge the search result from the secondary search with the first
toReturn = mergeReturns([
toReturn,
Expand All @@ -139,7 +144,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,
Expand All @@ -149,7 +153,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,
});
Expand All @@ -166,6 +170,7 @@ export const searchAfterAndBulkCreate = async ({
}),
]);

// TODO it's unclear why hits are guaranteed here; the type appears to be identical to the previous result which has more guards
// we are guaranteed to have searchResult hits at this point
// because we check before if the totalHits or
// searchResult.hits.hits.length is 0
Expand All @@ -186,14 +191,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(
Expand Down Expand Up @@ -228,6 +225,8 @@ export const searchAfterAndBulkCreate = async ({
tuple.maxSignals - signalsCreatedCount
);
}
const enrichedEvents = await enrichment(filteredEvents);
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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,
Expand All @@ -236,7 +235,7 @@ export const searchAfterAndBulkCreate = async ({
errors: bulkErrors,
} = await singleBulkCreate({
buildRuleMessage,
filteredEvents,
filteredEvents: enrichedEvents,
ruleParams,
services,
logger,
Expand Down
Loading