Skip to content

Commit

Permalink
[Security Solution][Detections] Reduce detection engine reliance on _…
Browse files Browse the repository at this point in the history 
…source (#89371) (#90287)

* First pass at switching rules to depend on fields instead of _source

* Fix tests

* Change operator: excluded logic so missing fields are allowlisted

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@marshallmain @kibanamachine
marshallmain and kibanamachine authored Feb 4, 2021
1 parent af940d5 commit 7478b45
Show file tree
Hide file tree
Showing 20 changed files with 162 additions and 71 deletions.
11 changes: 11 additions & 0 deletions x-pack/plugins/security_solution/server/lib/detection_engine/signals/__mocks__/es_results.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ export const sampleDocWithSortId = (
ip: destIp ?? '127.0.0.1',
},
},
fields: {
someKey: ['someValue'],
'@timestamp': ['2020-04-20T21:27:45+0000'],
'source.ip': ip ? (Array.isArray(ip) ? ip : [ip]) : ['127.0.0.1'],
'destination.ip': destIp ? (Array.isArray(destIp) ? destIp : [destIp]) : ['127.0.0.1'],
},
sort: ['1234567891111'],
});

Expand All @@ -185,6 +191,11 @@ export const sampleDocNoSortId = (
ip: ip ?? '127.0.0.1',
},
},
fields: {
someKey: ['someValue'],
'@timestamp': ['2020-04-20T21:27:45+0000'],
'source.ip': [ip ?? '127.0.0.1'],
},
sort: [],
});

Expand Down
41 changes: 36 additions & 5 deletions .../plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ describe('create_signals', () => {
],
},
},

fields: [
{
field: '*',
include_unmapped: true,
},
],
sort: [
{
'@timestamp': {
Expand Down Expand Up @@ -115,7 +120,12 @@ describe('create_signals', () => {
],
},
},

fields: [
{
field: '*',
include_unmapped: true,
},
],
sort: [
{
'@timestamp': {
Expand Down Expand Up @@ -175,7 +185,12 @@ describe('create_signals', () => {
],
},
},

fields: [
{
field: '*',
include_unmapped: true,
},
],
sort: [
{
'@timestamp': {
Expand Down Expand Up @@ -236,7 +251,12 @@ describe('create_signals', () => {
],
},
},

fields: [
{
field: '*',
include_unmapped: true,
},
],
sort: [
{
'@timestamp': {
Expand Down Expand Up @@ -296,7 +316,12 @@ describe('create_signals', () => {
],
},
},

fields: [
{
field: '*',
include_unmapped: true,
},
],
sort: [
{
'@timestamp': {
Expand Down Expand Up @@ -358,6 +383,12 @@ describe('create_signals', () => {
],
},
},
fields: [
{
field: '*',
include_unmapped: true,
},
],
aggregations: {
tags: {
terms: {
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ export const buildEventsSearchQuery = ({
],
},
},
fields: [
{
field: '*',
include_unmapped: true,
},
],
...(aggregations ? { aggregations } : {}),
sort: [
{
Expand Down
2 changes: 1 addition & 1 deletion ...curity_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('transformThresholdResultsToEcs', () => {
_id,
_index: 'test',
_source: {
'@timestamp': '2020-04-20T21:27:45+0000',
'@timestamp': ['2020-04-20T21:27:45+0000'],
threshold_result: {
count: 1,
value: '127.0.0.1',
Expand Down
6 changes: 3 additions & 3 deletions ...ns/security_solution/server/lib/detection_engine/signals/bulk_create_threshold_signals.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ const getTransformedHits = (
}

const source = {
'@timestamp': get(timestampOverride ?? '@timestamp', hit._source),
'@timestamp': get(timestampOverride ?? '@timestamp', hit.fields),
threshold_result: {
count: totalResults,
value: ruleId,
Expand Down Expand Up @@ -104,10 +104,10 @@ const getTransformedHits = (
}

const source = {
'@timestamp': get(timestampOverride ?? '@timestamp', hit._source),
'@timestamp': get(timestampOverride ?? '@timestamp', hit.fields),
threshold_result: {
count: docCount,
value: get(threshold.field, hit._source),
value: key,
},
};

Expand Down
8 changes: 4 additions & 4 deletions ..._solution/server/lib/detection_engine/signals/filters/create_field_and_set_tuples.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('filterEventsAgainstList', () => {
exceptionItem,
buildRuleMessage,
});
expect([...matchedSet]).toEqual([JSON.stringify('1.1.1.1')]);
expect([...matchedSet]).toEqual([JSON.stringify(['1.1.1.1'])]);
});

test('it returns two matched sets as a JSON.stringify() set from the "events"', async () => {
Expand All @@ -133,7 +133,7 @@ describe('filterEventsAgainstList', () => {
exceptionItem,
buildRuleMessage,
});
expect([...matchedSet]).toEqual([JSON.stringify('1.1.1.1'), JSON.stringify('2.2.2.2')]);
expect([...matchedSet]).toEqual([JSON.stringify(['1.1.1.1']), JSON.stringify(['2.2.2.2'])]);
});

test('it returns an array as a set as a JSON.stringify() array from the "events"', async () => {
Expand Down Expand Up @@ -282,7 +282,7 @@ describe('filterEventsAgainstList', () => {
exceptionItem,
buildRuleMessage,
});
expect([...matchedSet1]).toEqual([JSON.stringify('1.1.1.1'), JSON.stringify('2.2.2.2')]);
expect([...matchedSet2]).toEqual([JSON.stringify('3.3.3.3'), JSON.stringify('5.5.5.5')]);
expect([...matchedSet1]).toEqual([JSON.stringify(['1.1.1.1']), JSON.stringify(['2.2.2.2'])]);
expect([...matchedSet2]).toEqual([JSON.stringify(['3.3.3.3']), JSON.stringify(['5.5.5.5'])]);
});
});
8 changes: 4 additions & 4 deletions ...solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ describe('createSetToFilterAgainst', () => {
expect(listClient.searchListItemByValues).toHaveBeenCalledWith({
listId: 'list-123',
type: 'ip',
value: ['1.1.1.1'],
value: [['1.1.1.1']],
});
expect([...field]).toEqual([JSON.stringify('1.1.1.1')]);
expect([...field]).toEqual([JSON.stringify(['1.1.1.1'])]);
});

test('it returns 2 fields if the list returns 2 items', async () => {
Expand All @@ -81,9 +81,9 @@ describe('createSetToFilterAgainst', () => {
expect(listClient.searchListItemByValues).toHaveBeenCalledWith({
listId: 'list-123',
type: 'ip',
value: ['1.1.1.1', '2.2.2.2'],
value: [['1.1.1.1'], ['2.2.2.2']],
});
expect([...field]).toEqual([JSON.stringify('1.1.1.1'), JSON.stringify('2.2.2.2')]);
expect([...field]).toEqual([JSON.stringify(['1.1.1.1']), JSON.stringify(['2.2.2.2'])]);
});

test('it returns 0 fields if the field does not match up to a valid field within the event', async () => {
Expand Down
3 changes: 1 addition & 2 deletions ...rity_solution/server/lib/detection_engine/signals/filters/create_set_to_filter_against.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { get } from 'lodash/fp';
import { CreateSetToFilterAgainstOptions } from './types';

/**
Expand All @@ -31,7 +30,7 @@ export const createSetToFilterAgainst = async <T>({
buildRuleMessage,
}: CreateSetToFilterAgainstOptions<T>): Promise<Set<unknown>> => {
const valuesFromSearchResultField = events.reduce((acc, searchResultItem) => {
const valueField = get(field, searchResultItem._source);
const valueField = searchResultItem.fields ? searchResultItem.fields[field] : undefined;
if (valueField != null) {
acc.add(valueField);
}
Expand Down
10 changes: 5 additions & 5 deletions ...ugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('filterEvents', () => {
{
field: 'source.ip',
operator: 'included',
matchedSet: new Set([JSON.stringify('1.1.1.1')]),
matchedSet: new Set([JSON.stringify(['1.1.1.1'])]),
},
];
const field = filterEvents({
Expand All @@ -56,7 +56,7 @@ describe('filterEvents', () => {
{
field: 'source.ip',
operator: 'excluded',
matchedSet: new Set([JSON.stringify('1.1.1.1')]),
matchedSet: new Set([JSON.stringify(['1.1.1.1'])]),
},
];
const field = filterEvents({
Expand All @@ -72,7 +72,7 @@ describe('filterEvents', () => {
{
field: 'madeup.nonexistent', // field does not exist
operator: 'included',
matchedSet: new Set([JSON.stringify('1.1.1.1')]),
matchedSet: new Set([JSON.stringify(['1.1.1.1'])]),
},
];
const field = filterEvents({
Expand All @@ -88,12 +88,12 @@ describe('filterEvents', () => {
{
field: 'source.ip',
operator: 'included',
matchedSet: new Set([JSON.stringify('1.1.1.1')]),
matchedSet: new Set([JSON.stringify(['1.1.1.1'])]),
},
{
field: 'source.ip',
operator: 'excluded',
matchedSet: new Set([JSON.stringify('1.1.1.1')]),
matchedSet: new Set([JSON.stringify(['1.1.1.1'])]),
},
];

Expand Down
13 changes: 8 additions & 5 deletions ...ck/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { get } from 'lodash/fp';
import { SearchResponse } from '../../../types';
import { FilterEventsOptions } from './types';

Expand All @@ -22,13 +21,17 @@ export const filterEvents = <T>({
return events.filter((item) => {
return fieldAndSetTuples
.map((tuple) => {
const eventItem = get(tuple.field, item._source);
if (eventItem == null) {
return true;
} else if (tuple.operator === 'included') {
const eventItem = item.fields ? item.fields[tuple.field] : undefined;
if (tuple.operator === 'included') {
if (eventItem == null) {
return true;
}
// only create a signal if the event is not in the value list
return !tuple.matchedSet.has(JSON.stringify(eventItem));
} else if (tuple.operator === 'excluded') {
if (eventItem == null) {
return false;
}
// only create a signal if the event is in the value list
return tuple.matchedSet.has(JSON.stringify(eventItem));
} else {
Expand Down
16 changes: 8 additions & 8 deletions ...y_solution/server/lib/detection_engine/signals/filters/filter_events_against_list.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ describe('filterEventsAgainstList', () => {

// this call represents an exception list with a value list containing ['2.2.2.2', '4.4.4.4']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: '4.4.4.4' },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
{ ...getSearchListItemResponseMock(), value: ['4.4.4.4'] },
]);
// this call represents an exception list with a value list containing ['6.6.6.6']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '6.6.6.6' },
{ ...getSearchListItemResponseMock(), value: ['6.6.6.6'] },
]);

const res = await filterEventsAgainstList({
Expand Down Expand Up @@ -224,11 +224,11 @@ describe('filterEventsAgainstList', () => {

// this call represents an exception list with a value list containing ['2.2.2.2', '4.4.4.4']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
]);
// this call represents an exception list with a value list containing ['6.6.6.6']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '6.6.6.6' },
{ ...getSearchListItemResponseMock(), value: ['6.6.6.6'] },
]);

const res = await filterEventsAgainstList({
Expand Down Expand Up @@ -283,11 +283,11 @@ describe('filterEventsAgainstList', () => {

// this call represents an exception list with a value list containing ['2.2.2.2']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
]);
// this call represents an exception list with a value list containing ['4.4.4.4']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValueOnce([
{ ...getSearchListItemResponseMock(), value: '4.4.4.4' },
{ ...getSearchListItemResponseMock(), value: ['4.4.4.4'] },
]);

const res = await filterEventsAgainstList({
Expand Down Expand Up @@ -365,7 +365,7 @@ describe('filterEventsAgainstList', () => {

// this call represents an exception list with a value list containing ['2.2.2.2', '4.4.4.4']
(listClient.searchListItemByValues as jest.Mock).mockResolvedValue([
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
]);

const res = await filterEventsAgainstList({
Expand Down
6 changes: 6 additions & 0 deletions ...k/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ export const findThresholdSignals = async ({
},
},
],
fields: [
{
field: '*',
include_unmapped: true,
},
],
size: 1,
},
},
Expand Down
14 changes: 7 additions & 7 deletions ...ns/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,9 @@ describe('searchAfterAndBulkCreate', () => {

test('should return success when all search results are in the allowlist and with sortId present', async () => {
const searchListItems: SearchListItemArraySchema = [
{ ...getSearchListItemResponseMock(), value: '1.1.1.1' },
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: '3.3.3.3' },
{ ...getSearchListItemResponseMock(), value: ['1.1.1.1'] },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
{ ...getSearchListItemResponseMock(), value: ['3.3.3.3'] },
];
listClient.searchListItemByValues = jest.fn().mockResolvedValue(searchListItems);
const sampleParams = sampleRuleAlertParams(30);
Expand Down Expand Up @@ -374,10 +374,10 @@ describe('searchAfterAndBulkCreate', () => {

test('should return success when all search results are in the allowlist and no sortId present', async () => {
const searchListItems: SearchListItemArraySchema = [
{ ...getSearchListItemResponseMock(), value: '1.1.1.1' },
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: '2.2.2.2' },
{ ...getSearchListItemResponseMock(), value: ['1.1.1.1'] },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
{ ...getSearchListItemResponseMock(), value: ['2.2.2.2'] },
];

listClient.searchListItemByValues = jest.fn().mockResolvedValue(searchListItems);
Expand Down
Loading

0 comments on commit 7478b45

Please sign in to comment.