Skip to content

Commit

Permalink
[SIEM] [Detections] Fixes faulty circuit breaker (#71999)
Browse files Browse the repository at this point in the history 
* removes useSortIds which was leftover from a previous attempt at implementing gap detection mitigation code. This only showed up because I modified the count variable used to determine when we hit maxSignals from utilizing the searchResult hits length to using the count of bulk created items (signals indexed) in this commit 56de45d

* removes logs and fixes if statement ordering

* adds tests, increases code coverage for search after and bulk create function, updates log statements

* update tests after rebase onto master

* clean up if statements

* fix test data

* merge conflicts are hard
  • Loading branch information
@dhurley14
dhurley14 authored Jul 20, 2020
1 parent 4ccf1ae commit b9413cf
Show file tree
Hide file tree
Showing 4 changed files with 351 additions and 76 deletions.
42 changes: 37 additions & 5 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 @@ -56,30 +56,37 @@ export const sampleRuleAlertParams = (
exceptionsList: getListArrayMock(),
});

export const sampleDocNoSortId = (someUuid: string = sampleIdGuid): SignalSourceHit => ({
export const sampleDocNoSortIdNoVersion = (someUuid: string = sampleIdGuid): SignalSourceHit => ({
_index: 'myFakeSignalIndex',
_type: 'doc',
_score: 100,
_version: 1,
_id: someUuid,
_source: {
someKey: 'someValue',
'@timestamp': '2020-04-20T21:27:45+0000',
},
});

export const sampleDocNoSortIdNoVersion = (someUuid: string = sampleIdGuid): SignalSourceHit => ({
export const sampleDocWithSortId = (
someUuid: string = sampleIdGuid,
ip?: string
): SignalSourceHit => ({
_index: 'myFakeSignalIndex',
_type: 'doc',
_score: 100,
_version: 1,
_id: someUuid,
_source: {
someKey: 'someValue',
'@timestamp': '2020-04-20T21:27:45+0000',
source: {
ip: ip ?? '127.0.0.1',
},
},
sort: ['1234567891111'],
});

export const sampleDocWithSortId = (
export const sampleDocNoSortId = (
someUuid: string = sampleIdGuid,
ip?: string
): SignalSourceHit => ({
Expand All @@ -95,7 +102,7 @@ export const sampleDocWithSortId = (
ip: ip ?? '127.0.0.1',
},
},
sort: ['1234567891111'],
sort: [],
});

export const sampleEmptyDocSearchResults = (): SignalSearchResponse => ({
Expand All @@ -116,6 +123,8 @@ export const sampleEmptyDocSearchResults = (): SignalSearchResponse => ({

export const sampleDocWithAncestors = (): SignalSearchResponse => {
const sampleDoc = sampleDocNoSortId();
delete sampleDoc.sort;
delete sampleDoc._source.source;
sampleDoc._source.signal = {
parent: {
rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
Expand Down Expand Up @@ -317,6 +326,29 @@ export const repeatedSearchResultsWithSortId = (
},
});

export const repeatedSearchResultsWithNoSortId = (
total: number,
pageSize: number,
guids: string[],
ips?: string[]
) => ({
took: 10,
timed_out: false,
_shards: {
total: 10,
successful: 10,
failed: 0,
skipped: 0,
},
hits: {
total,
max_score: 100,
hits: Array.from({ length: pageSize }).map((x, index) => ({
...sampleDocNoSortId(guids[index], ips ? ips[index] : '127.0.0.1'),
})),
},
});

export const sampleDocSearchResultsWithSortId = (
someUuid: string = sampleIdGuid
): SignalSearchResponse => ({
Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ describe('buildBulkBody', () => {

test('bulk body builds well-defined body', () => {
const sampleParams = sampleRuleAlertParams();
const doc = sampleDocNoSortId();
delete doc._source.source;
const fakeSignalSourceHit = buildBulkBody({
doc: sampleDocNoSortId(),
doc,
ruleParams: sampleParams,
id: sampleRuleGuid,
name: 'rule-name',
Expand Down Expand Up @@ -107,6 +109,7 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with', () => {
const sampleParams = sampleRuleAlertParams();
const doc = sampleDocNoSortId();
delete doc._source.source;
doc._source.event = {
action: 'socket_opened',
module: 'system',
Expand Down Expand Up @@ -208,6 +211,7 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with but no kind information', () => {
const sampleParams = sampleRuleAlertParams();
const doc = sampleDocNoSortId();
delete doc._source.source;
doc._source.event = {
action: 'socket_opened',
module: 'system',
Expand Down Expand Up @@ -307,6 +311,7 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with with only kind information', () => {
const sampleParams = sampleRuleAlertParams();
const doc = sampleDocNoSortId();
delete doc._source.source;
doc._source.event = {
kind: 'event',
};
Expand Down
207 changes: 207 additions & 0 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 @@ -11,6 +11,7 @@ import {
sampleRuleGuid,
mockLogger,
repeatedSearchResultsWithSortId,
repeatedSearchResultsWithNoSortId,
sampleDocSearchResultsNoSortIdNoHits,
} from './__mocks__/es_results';
import { searchAfterAndBulkCreate } from './search_after_bulk_create';
Expand Down Expand Up @@ -356,6 +357,212 @@ describe('searchAfterAndBulkCreate', () => {
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
});

test('should return success when all search results are in the allowlist and with sortId present', async () => {
listClient.getListItemByValues = jest
.fn()
.mockResolvedValue([{ value: '1.1.1.1' }, { value: '2.2.2.2' }, { value: '3.3.3.3' }]);
const sampleParams = sampleRuleAlertParams(30);
mockService.callCluster
.mockResolvedValueOnce(
repeatedSearchResultsWithSortId(4, 4, someGuids.slice(0, 3), [
'1.1.1.1',
'2.2.2.2',
'2.2.2.2',
'2.2.2.2',
])
)
.mockResolvedValueOnce(sampleDocSearchResultsNoSortIdNoHits());

const exceptionItem = getExceptionListItemSchemaMock();
exceptionItem.entries = [
{
field: 'source.ip',
operator: 'included',
type: 'list',
list: {
id: 'ci-badguys.txt',
type: 'ip',
},
},
];
const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({
ruleParams: sampleParams,
gap: null,
previousStartedAt: new Date(),
listClient,
exceptionsList: [exceptionItem],
services: mockService,
logger: mockLogger,
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(success).toEqual(true);
expect(mockService.callCluster).toHaveBeenCalledTimes(2);
expect(createdSignalsCount).toEqual(0); // should not create any signals because all events were in the allowlist
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
});

test('should return success when all search results are in the allowlist and no sortId present', async () => {
listClient.getListItemByValues = jest
.fn()
.mockResolvedValue([{ value: '1.1.1.1' }, { value: '2.2.2.2' }, { value: '3.3.3.3' }]);
const sampleParams = sampleRuleAlertParams(30);
mockService.callCluster.mockResolvedValueOnce(
repeatedSearchResultsWithNoSortId(4, 4, someGuids.slice(0, 3), [
'1.1.1.1',
'2.2.2.2',
'2.2.2.2',
'2.2.2.2',
])
);

const exceptionItem = getExceptionListItemSchemaMock();
exceptionItem.entries = [
{
field: 'source.ip',
operator: 'included',
type: 'list',
list: {
id: 'ci-badguys.txt',
type: 'ip',
},
},
];
const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({
ruleParams: sampleParams,
gap: null,
previousStartedAt: new Date(),
listClient,
exceptionsList: [exceptionItem],
services: mockService,
logger: mockLogger,
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(success).toEqual(true);
expect(mockService.callCluster).toHaveBeenCalledTimes(1);
expect(createdSignalsCount).toEqual(0); // should not create any signals because all events were in the allowlist
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
// I don't like testing log statements since logs change but this is the best
// way I can think of to ensure this section is getting hit with this test case.
expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[7][0]).toContain(
'sortIds was empty on searchResult'
);
});

test('should return success when no sortId present but search results are in the allowlist', async () => {
const sampleParams = sampleRuleAlertParams(30);
mockService.callCluster
.mockResolvedValueOnce(repeatedSearchResultsWithNoSortId(4, 4, someGuids.slice(0, 3)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
fakeItemValue: 'fakeItemKey',
},
{
create: {
status: 201,
},
},
{
create: {
status: 201,
},
},
{
create: {
status: 201,
},
},
{
create: {
status: 201,
},
},
],
});

const exceptionItem = getExceptionListItemSchemaMock();
exceptionItem.entries = [
{
field: 'source.ip',
operator: 'included',
type: 'list',
list: {
id: 'ci-badguys.txt',
type: 'ip',
},
},
];
const { success, createdSignalsCount, lastLookBackDate } = await searchAfterAndBulkCreate({
ruleParams: sampleParams,
gap: null,
previousStartedAt: new Date(),
listClient,
exceptionsList: [exceptionItem],
services: mockService,
logger: mockLogger,
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(success).toEqual(true);
expect(mockService.callCluster).toHaveBeenCalledTimes(2);
expect(createdSignalsCount).toEqual(4); // should not create any signals because all events were in the allowlist
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
// I don't like testing log statements since logs change but this is the best
// way I can think of to ensure this section is getting hit with this test case.
expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[12][0]).toContain(
'sortIds was empty on filteredEvents'
);
});

test('should return success when no exceptions list provided', async () => {
const sampleParams = sampleRuleAlertParams(30);
mockService.callCluster
Expand Down
Loading

0 comments on commit b9413cf

Please sign in to comment.