Skip to content

Commit

Permalink
[Security Solutions][Detection Engine] Fixes critical bug where an in…
Browse files Browse the repository at this point in the history
…valid/empty mapping can cause a match all (#80415)

## Summary

Critical bug where if you have an invalid/empty mapping this can cause the threat match to end up matching against everything rather than matching against an empty set.

Added integration end to end tests to exercise both this case and the positive case where regular mappings should match against 10+ things.

If you want to test this manually you can do the following:

First find a field that doesn't have any data in it such as `url.extension` using timeline

<img width="1160" alt="Screen Shot 2020-10-13 at 2 54 17 PM" src="https://user-images.githubusercontent.com/1151048/95915285-2c890500-0d64-11eb-8a78-3a12caf280f5.png">

Then create the rule with it using any other matching field:
<img width="1036" alt="Screen Shot 2020-10-13 at 2 53 39 PM" src="https://user-images.githubusercontent.com/1151048/95915319-36ab0380-0d64-11eb-9d54-65978279586c.png">

Then run the rule and see that you have signals when there should be zero signals happening.
 
### 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
  • Loading branch information
FrankHassanabad authored Oct 14, 2020
1 parent 2955584 commit 2db9542
Show file tree
Hide file tree
Showing 5 changed files with 352 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,74 @@ export const getThreatMatchingSchemaMock = (anchorDate: string = ANCHOR_DATE): R
};
};

/**
* Useful for e2e backend tests where it doesn't have date time and other
* server side properties attached to it.
*/
export const getThreatMatchingSchemaPartialMock = (): Partial<RulesSchema> => {
return {
author: [],
created_by: 'elastic',
description: 'Detecting root and admin users',
enabled: true,
false_positives: [],
from: 'now-6m',
immutable: false,
interval: '5m',
rule_id: 'rule-1',
output_index: '.siem-signals-default',
max_signals: 100,
risk_score: 55,
risk_score_mapping: [],
name: 'Query with a rule id',
references: [],
severity: 'high',
severity_mapping: [],
updated_by: 'elastic',
tags: [],
to: 'now',
type: 'threat_match',
threat: [],
version: 1,
exceptions_list: [],
actions: [],
throttle: 'no_actions',
query: 'user.name: root or user.name: admin',
language: 'kuery',
threat_query: '*:*',
threat_index: ['list-index'],
threat_mapping: [
{
entries: [
{
field: 'host.name',
value: 'host.name',
type: 'mapping',
},
],
},
],
threat_filters: [
{
bool: {
must: [
{
query_string: {
query: 'host.name: linux',
analyze_wildcard: true,
time_zone: 'Zulu',
},
},
],
filter: [],
should: [],
must_not: [],
},
},
],
};
};

export const getRulesEqlSchemaMock = (anchorDate: string = ANCHOR_DATE): RulesSchema => {
return {
...getRulesSchemaMock(anchorDate),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { SearchResponse } from 'elasticsearch';
import { getThreatList } from './get_threat_list';
import { buildThreatMappingFilter } from './build_threat_mapping_filter';

import { getFilter } from '../get_filter';
import { searchAfterAndBulkCreate } from '../search_after_bulk_create';
import { CreateThreatSignalOptions, ThreatListItem } from './types';
import { CreateThreatSignalOptions, ThreatSignalResults } from './types';
import { combineResults } from './utils';
import { SearchAfterAndBulkCreateReturnType } from '../types';

export const createThreatSignal = async ({
threatMapping,
Expand Down Expand Up @@ -51,68 +49,67 @@ export const createThreatSignal = async ({
name,
currentThreatList,
currentResult,
}: CreateThreatSignalOptions): Promise<{
threatList: SearchResponse<ThreatListItem>;
results: SearchAfterAndBulkCreateReturnType;
}> => {
const threatFilter = buildThreatMappingFilter({
threatMapping,
threatList: currentThreatList,
});

const esFilter = await getFilter({
type,
filters: [...filters, threatFilter],
language,
query,
savedId,
services,
index: inputIndex,
lists: exceptionItems,
});

const newResult = await searchAfterAndBulkCreate({
gap,
previousStartedAt,
listClient,
exceptionsList: exceptionItems,
ruleParams: params,
services,
logger,
eventsTelemetry,
id: alertId,
inputIndexPattern: inputIndex,
signalsIndex: outputIndex,
filter: esFilter,
actions,
name,
createdBy,
createdAt,
updatedBy,
updatedAt,
interval,
enabled,
pageSize: searchAfterSize,
refresh,
tags,
throttle,
buildRuleMessage,
});

const results = combineResults(currentResult, newResult);
const searchAfter = currentThreatList.hits.hits[currentThreatList.hits.hits.length - 1].sort;

}: CreateThreatSignalOptions): Promise<ThreatSignalResults> => {
const threatList = await getThreatList({
callCluster: services.callCluster,
exceptionItems,
query: threatQuery,
language: threatLanguage,
threatFilters,
index: threatIndex,
searchAfter,
searchAfter: currentThreatList.hits.hits[currentThreatList.hits.hits.length - 1].sort,
sortField: undefined,
sortOrder: undefined,
});

return { threatList, results };
const threatFilter = buildThreatMappingFilter({
threatMapping,
threatList: currentThreatList,
});

if (threatFilter.query.bool.should.length === 0) {
// empty threat list and we do not want to return everything as being
// a hit so opt to return the existing result.
return { threatList, results: currentResult };
} else {
const esFilter = await getFilter({
type,
filters: [...filters, threatFilter],
language,
query,
savedId,
services,
index: inputIndex,
lists: exceptionItems,
});
const newResult = await searchAfterAndBulkCreate({
gap,
previousStartedAt,
listClient,
exceptionsList: exceptionItems,
ruleParams: params,
services,
logger,
eventsTelemetry,
id: alertId,
inputIndexPattern: inputIndex,
signalsIndex: outputIndex,
filter: esFilter,
actions,
name,
createdBy,
createdAt,
updatedBy,
updatedAt,
interval,
enabled,
pageSize: searchAfterSize,
refresh,
tags,
throttle,
buildRuleMessage,
});
const results = combineResults(currentResult, newResult);
return { threatList, results };
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ export interface CreateThreatSignalOptions {
currentResult: SearchAfterAndBulkCreateReturnType;
}

export interface ThreatSignalResults {
threatList: SearchResponse<ThreatListItem>;
results: SearchAfterAndBulkCreateReturnType;
}

export interface BuildThreatMappingFilterOptions {
threatMapping: ThreatMapping;
threatList: SearchResponse<ThreatListItem>;
Expand Down
Loading

0 comments on commit 2db9542

Please sign in to comment.