Skip to content

Commit

Permalink
[Security Solution] Adds logging and performance fan out API for thre…
Browse files Browse the repository at this point in the history
…at/Indicator matching (elastic#82546)

## Summary

* Adds logging output for trouble shooting
* Adds an API to be able to configure how many concurrent searches and how many items per search to use

API additions are these two switches:

```
concurrent_searches
items_per_search
```

When you create a rule. You can use the following example to post one or to change the settings to see the performance impact:

```ts
./post_rule.sh ./rules/queries/query_with_threat_mapping_perf.json
```

Without using these two experimental API settings, the functionality is the same as the existing algorithm and only advanced users will be able to set the additional REST settings through this API. If you use the front end after setting the settings, the settings will be reset as that's how the forms code currently works and this will not preserve the settings if afterwards a rule is edited/changed.

Both these API settings should be considered experimental and potentially breakable as we figure out the best performance strategies for indicator matching.

### Checklist

- [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 committed Nov 7, 2020
1 parent c2d04d3 commit b4d85cc
Show file tree
Hide file tree
Showing 48 changed files with 981 additions and 144 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import {
} from '../common/schemas';
import {
threat_index,
concurrent_searches,
items_per_search,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -130,6 +132,8 @@ export const addPrepackagedRulesSchema = t.intersection([
threat_query, // defaults to "undefined" if not set during decode
threat_index, // defaults to "undefined" if not set during decode
threat_language, // defaults "undefined" if not set during decode
concurrent_searches, // defaults to "undefined" if not set during decode
items_per_search, // defaults to "undefined" if not set during decode
})
),
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1702,5 +1702,23 @@ describe('create rules schema', () => {
expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(expected);
});

test('You can set a threat query, index, mapping, filters, concurrent_searches, items_per_search with a when creating a rule', () => {
const payload: CreateRulesSchema = {
...getCreateThreatMatchRulesSchemaMock(),
concurrent_searches: 10,
items_per_search: 10,
};
const decoded = createRulesSchema.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const expected: CreateRulesSchemaDecoded = {
...getCreateThreatMatchRulesSchemaDecodedMock(),
concurrent_searches: 10,
items_per_search: 10,
};
expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(expected);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import {
} from '../common/schemas';
import {
threat_index,
concurrent_searches,
items_per_search,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -126,6 +128,8 @@ export const createRulesSchema = t.intersection([
threat_filters, // defaults to "undefined" if not set during decode
threat_index, // defaults to "undefined" if not set during decode
threat_language, // defaults "undefined" if not set during decode
concurrent_searches, // defaults "undefined" if not set during decode
items_per_search, // defaults "undefined" if not set during decode
})
),
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,36 @@ describe('create_rules_type_dependents', () => {
const errors = createRuleValidateTypeDependents(schema);
expect(errors).toEqual([]);
});

test('validates that both "items_per_search" and "concurrent_searches" works when together', () => {
const schema: CreateRulesSchema = {
...getCreateThreatMatchRulesSchemaMock(),
concurrent_searches: 10,
items_per_search: 10,
};
const errors = createRuleValidateTypeDependents(schema);
expect(errors).toEqual([]);
});

test('does NOT validate when only "items_per_search" is present', () => {
const schema: CreateRulesSchema = {
...getCreateThreatMatchRulesSchemaMock(),
items_per_search: 10,
};
const errors = createRuleValidateTypeDependents(schema);
expect(errors).toEqual([
'when "items_per_search" exists, "concurrent_searches" must also exist',
]);
});

test('does NOT validate when only "concurrent_searches" is present', () => {
const schema: CreateRulesSchema = {
...getCreateThreatMatchRulesSchemaMock(),
concurrent_searches: 10,
};
const errors = createRuleValidateTypeDependents(schema);
expect(errors).toEqual([
'when "concurrent_searches" exists, "items_per_search" must also exist',
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,23 @@ export const validateThreshold = (rule: CreateRulesSchema): string[] => {
export const validateThreatMapping = (rule: CreateRulesSchema): string[] => {
let errors: string[] = [];
if (isThreatMatchRule(rule.type)) {
if (!rule.threat_mapping) {
if (rule.threat_mapping == null) {
errors = ['when "type" is "threat_match", "threat_mapping" is required', ...errors];
} else if (rule.threat_mapping.length === 0) {
errors = ['threat_mapping" must have at least one element', ...errors];
}
if (!rule.threat_query) {
if (rule.threat_query == null) {
errors = ['when "type" is "threat_match", "threat_query" is required', ...errors];
}
if (!rule.threat_index) {
if (rule.threat_index == null) {
errors = ['when "type" is "threat_match", "threat_index" is required', ...errors];
}
if (rule.concurrent_searches == null && rule.items_per_search != null) {
errors = ['when "items_per_search" exists, "concurrent_searches" must also exist', ...errors];
}
if (rule.concurrent_searches != null && rule.items_per_search == null) {
errors = ['when "concurrent_searches" exists, "items_per_search" must also exist', ...errors];
}
}
return errors;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ import {
} from '../common/schemas';
import {
threat_index,
items_per_search,
concurrent_searches,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -149,6 +151,8 @@ export const importRulesSchema = t.intersection([
threat_query, // defaults to "undefined" if not set during decode
threat_index, // defaults to "undefined" if not set during decode
threat_language, // defaults "undefined" if not set during decode
concurrent_searches, // defaults to "undefined" if not set during decode
items_per_search, // defaults to "undefined" if not set during decode
})
),
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ import {
} from '../common/schemas';
import {
threat_index,
concurrent_searches,
items_per_search,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -109,6 +111,8 @@ export const patchRulesSchema = t.exact(
threat_filters,
threat_mapping,
threat_language,
concurrent_searches,
items_per_search,
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ import {
} from '../common/schemas';
import {
threat_index,
concurrent_searches,
items_per_search,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -134,6 +136,8 @@ export const updateRulesSchema = t.intersection([
threat_filters, // defaults to "undefined" if not set during decode
threat_index, // defaults to "undefined" if not set during decode
threat_language, // defaults "undefined" if not set during decode
concurrent_searches, // defaults to "undefined" if not set during decode
items_per_search, // defaults to "undefined" if not set during decode
})
),
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,9 @@ describe('rules_schema', () => {
expect(fields).toEqual(expected);
});

test('should return 5 fields for a rule of type "threat_match"', () => {
test('should return 8 fields for a rule of type "threat_match"', () => {
const fields = addThreatMatchFields({ type: 'threat_match' });
expect(fields.length).toEqual(6);
expect(fields.length).toEqual(8);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ import {
} from '../common/schemas';
import {
threat_index,
concurrent_searches,
items_per_search,
threat_query,
threat_filters,
threat_mapping,
Expand Down Expand Up @@ -144,6 +146,8 @@ export const dependentRulesSchema = t.partial({
threat_filters,
threat_index,
threat_query,
concurrent_searches,
items_per_search,
threat_mapping,
threat_language,
});
Expand Down Expand Up @@ -282,6 +286,12 @@ export const addThreatMatchFields = (typeAndTimelineOnly: TypeAndTimelineOnly):
t.exact(t.partial({ threat_language: dependentRulesSchema.props.threat_language })),
t.exact(t.partial({ threat_filters: dependentRulesSchema.props.threat_filters })),
t.exact(t.partial({ saved_id: dependentRulesSchema.props.saved_id })),
t.exact(t.partial({ concurrent_searches: dependentRulesSchema.props.concurrent_searches })),
t.exact(
t.partial({
items_per_search: dependentRulesSchema.props.items_per_search,
})
),
];
} else {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/

import {
concurrent_searches,
items_per_search,
ThreatMapping,
threatMappingEntries,
ThreatMappingEntries,
Expand Down Expand Up @@ -33,7 +35,7 @@ describe('threat_mapping', () => {
expect(message.schema).toEqual(payload);
});

test('it should NOT validate an extra entry item', () => {
test('it should fail validation with an extra entry item', () => {
const payload: ThreatMappingEntries & Array<{ extra: string }> = [
{
field: 'field.one',
Expand All @@ -50,7 +52,7 @@ describe('threat_mapping', () => {
expect(message.schema).toEqual({});
});

test('it should NOT validate a non string', () => {
test('it should fail validation with a non string', () => {
const payload = ([
{
field: 5,
Expand All @@ -66,7 +68,7 @@ describe('threat_mapping', () => {
expect(message.schema).toEqual({});
});

test('it should NOT validate a wrong type', () => {
test('it should fail validation with a wrong type', () => {
const payload = ([
{
field: 'field.one',
Expand Down Expand Up @@ -107,7 +109,7 @@ describe('threat_mapping', () => {
});
});

test('it should NOT validate an extra key', () => {
test('it should fail validate with an extra key', () => {
const payload: ThreatMapping & Array<{ extra: string }> = [
{
entries: [
Expand All @@ -129,7 +131,7 @@ describe('threat_mapping', () => {
expect(message.schema).toEqual({});
});

test('it should NOT validate an extra inner entry', () => {
test('it should fail validate with an extra inner entry', () => {
const payload: ThreatMapping & Array<{ entries: Array<{ extra: string }> }> = [
{
entries: [
Expand All @@ -151,7 +153,7 @@ describe('threat_mapping', () => {
expect(message.schema).toEqual({});
});

test('it should NOT validate an extra inner entry with the wrong data type', () => {
test('it should fail validate with an extra inner entry with the wrong data type', () => {
const payload = ([
{
entries: [
Expand All @@ -173,4 +175,48 @@ describe('threat_mapping', () => {
]);
expect(message.schema).toEqual({});
});

test('it should fail validation when concurrent_searches is < 0', () => {
const payload = -1;
const decoded = concurrent_searches.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "PositiveIntegerGreaterThanZero"',
]);
expect(message.schema).toEqual({});
});

test('it should fail validation when concurrent_searches is 0', () => {
const payload = 0;
const decoded = concurrent_searches.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "0" supplied to "PositiveIntegerGreaterThanZero"',
]);
expect(message.schema).toEqual({});
});

test('it should fail validation when items_per_search is 0', () => {
const payload = 0;
const decoded = items_per_search.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "0" supplied to "PositiveIntegerGreaterThanZero"',
]);
expect(message.schema).toEqual({});
});

test('it should fail validation when items_per_search is < 0', () => {
const payload = -1;
const decoded = items_per_search.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "PositiveIntegerGreaterThanZero"',
]);
expect(message.schema).toEqual({});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import * as t from 'io-ts';
import { language } from '../common/schemas';
import { NonEmptyString } from './non_empty_string';
import { PositiveIntegerGreaterThanZero } from './positive_integer_greater_than_zero';

export const threat_query = t.string;
export type ThreatQuery = t.TypeOf<typeof threat_query>;
Expand Down Expand Up @@ -55,3 +56,13 @@ export const threat_language = t.union([language, t.undefined]);
export type ThreatLanguage = t.TypeOf<typeof threat_language>;
export const threatLanguageOrUndefined = t.union([threat_language, t.undefined]);
export type ThreatLanguageOrUndefined = t.TypeOf<typeof threatLanguageOrUndefined>;

export const concurrent_searches = PositiveIntegerGreaterThanZero;
export type ConcurrentSearches = t.TypeOf<typeof concurrent_searches>;
export const concurrentSearchesOrUndefined = t.union([concurrent_searches, t.undefined]);
export type ConcurrentSearchesOrUndefined = t.TypeOf<typeof concurrentSearchesOrUndefined>;

export const items_per_search = PositiveIntegerGreaterThanZero;
export type ItemsPerSearch = t.TypeOf<typeof concurrent_searches>;
export const itemsPerSearchOrUndefined = t.union([items_per_search, t.undefined]);
export type ItemsPerSearchOrUndefined = t.TypeOf<typeof itemsPerSearchOrUndefined>;
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const THREAT_MATCH_INDEX_HELPER_TEXT = i18n.translate(
export const THREAT_MATCH_REQUIRED = i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepDefineRule.customThreatQueryFieldRequiredError',
{
defaultMessage: 'At least one threat match is required.',
defaultMessage: 'At least one indicator match is required.',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,8 @@ export const getResult = (): RuleAlertType => ({
note: '# Investigative notes',
version: 1,
exceptionsList: getListArrayMock(),
concurrentSearches: undefined,
itemsPerSearch: undefined,
},
createdAt: new Date('2019-12-13T16:40:33.400Z'),
updatedAt: new Date('2019-12-13T16:40:33.400Z'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ export const createRulesBulkRoute = (router: IRouter, ml: SetupPlugins['ml']) =>
threat_mapping: threatMapping,
threat_query: threatQuery,
threat_language: threatLanguage,
concurrent_searches: concurrentSearches,
items_per_search: itemsPerSearch,
threshold,
throttle,
timestamp_override: timestampOverride,
Expand Down Expand Up @@ -193,6 +195,8 @@ export const createRulesBulkRoute = (router: IRouter, ml: SetupPlugins['ml']) =>
threatQuery,
threatIndex,
threatLanguage,
concurrentSearches,
itemsPerSearch,
threshold,
timestampOverride,
references,
Expand Down
Loading

0 comments on commit b4d85cc

Please sign in to comment.