Skip to content

Commit ead4384

Browse files
[Security Solutions][Detection Engine] Adds ability to ignore fields during alert indexing and a workaround for an EQL bug (#110927) (#111174)
## Summary Adds a workaround for EQL bug: elastic/elasticsearch#77152 Adds the safety feature mentioned here: #110802 Adds the ability to ignore particular [fields](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#search-fields-param) when the field is merged with [_source](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#source-filtering). Also fixes an EQL bug where EQL is introducing the meta field of `_ignored` within the fields and causing documents to not be indexable when we merge with the fields from EQL. Alerting document creation uses the fields API to get [runtime field](https://www.elastic.co/guide/en/elasticsearch/reference/current/runtime.html), [constant keyword](https://www.elastic.co/guide/en/elasticsearch/reference/master/keyword.html#constant-keyword-field-type), etc... that are only available within the [fields API](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#search-fields-param) and then merges the field values not found within the `_source` document with the `_source` document and then finally indexes this merged document as an alert document. This fix/ability is a "safety feature" in that if a problematic [runtime field](https://www.elastic.co/guide/en/elasticsearch/reference/current/runtime.html), [constant keyword](https://www.elastic.co/guide/en/elasticsearch/reference/master/keyword.html#constant-keyword-field-type) is discovered or another bug along the stack we can set a `kibana.yml` key/value pair to ignore the problematic field. This _WILL NOT_ remove problematic fields from the `_source` document. This will only ignore problematic constant keyword, runtime fields, aliases, or anything else found in the fields API that is causing merge issues. This PR: * Adds a `alertIgnoreFields` `kibana.yml` array key with a default of an empty array if not specified. * Plumbs the `alertIgnoreFields` through the stack and into the fields/_source merge strategies of `missingFields` and `allFields` * Adds a temporary `isEqlBug77152` where it hard codes an ignore of `_ignored` until the EQL problem is fixed and then we will remove the workaround * Adds unit tests * Adds e2e tests which covers the described use cases above. The `alertIgnoreFields` key/value within `kibana.yml` if set should be an array of strings of each field you want to ignore. This can also contain regular expressions as long as they are of the form, `"/regex/"` in the array. Example if you want to ignore fields that are problematic called "host.name" and then one in which you want to ignore all fields that start with "user." using a regular expression: ```yml xpack.securitySolution.alertIgnoreFields: ['host.name', '/user\..*/'] ``` Although there are e2e tests which exercise the use cases... If you want to manual test the EQL bug fix you would add these documents in dev tools: ```json # Delete and add a mapping with a small ignore_above. DELETE eql-issue-ignore-fields-delme PUT eql-issue-ignore-fields-delme { "mappings" : { "dynamic": "strict", "properties" : { "@timestamp": { "type": "date" }, "some_keyword" : { "ignore_above": 5, "type" : "keyword" }, "other_keyword" : { "ignore_above": 10, "type" : "keyword" } } } } # Add a single document with one field that will be truncated and a second that will not. PUT eql-issue-ignore-fields-delme/_doc/1 { "@timestamp": "2021-09-02T04:13:05.626Z", "some_keyword": "longer than normal", "other_keyword": "normal" } ``` Then create an alert which queries everything from it: <img width="1155" alt="Screen Shot 2021-09-01 at 10 15 06 PM" src="https://user-images.githubusercontent.com/1151048/131781042-faa424cf-65a5-4ebb-b801-3f188940c81d.png"> and ensure signals are created: <img width="2214" alt="Screen Shot 2021-09-01 at 10 30 18 PM" src="https://user-images.githubusercontent.com/1151048/131782069-b9ab959c-f22d-44d5-baf0-561fe349c037.png"> To test the manual exclusions of any other problematic fields, create any index which has runtime fields or `constant keywords` but does not have anything within the `_source` document using dev tools. For example you can use `constant keyword` like so ```json PUT constant-keywords-deleme { "mappings": { "dynamic": "strict", "properties": { "@timestamp": { "type": "date" }, "testing_ignored": { "properties": { "constant": { "type": "constant_keyword", "value": "constant_value" } } }, "testing_regex": { "type": "constant_keyword", "value": "constant_value" }, "normal_constant": { "type": "constant_keyword", "value": "constant_value" }, "small_field": { "type": "keyword", "ignore_above": 10 } } } } PUT constant-keywords-deleme/_doc/1 { "@timestamp": "2021-09-02T04:20:01.760Z" } ``` Set in your `kibana.yml` the key/value of: ```yml xpack.securitySolution.alertIgnoreFields: ['testing_ignored.constant', '/.*_regex/'] ``` Setup a rule to run: <img width="1083" alt="Screen Shot 2021-09-01 at 10 23 23 PM" src="https://user-images.githubusercontent.com/1151048/131781696-fea0d421-836f-465c-9be6-5289fbb622a4.png"> Once it runs you should notice that the constant values for testing are not on the signals table since it only typically exists in the fields API: <img width="1166" alt="Screen Shot 2021-09-01 at 10 26 16 PM" src="https://user-images.githubusercontent.com/1151048/131781782-1684fb1d-bed9-4cf0-be9a-0abe1f0f34d1.png"> But the normal one still exists: <img width="1136" alt="Screen Shot 2021-09-01 at 10 26 31 PM" src="https://user-images.githubusercontent.com/1151048/131781827-5450c693-de9e-4285-b082-9f7a2cbd5d07.png"> If you change the `xpack.securitySolution.alertIgnoreFields` by removing it and re-generate the signals you will see these values added back. ### 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 - [x] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/master/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) # Conflicts: # src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/utils/build_bulk_body.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/factories/wrap_hits_factory.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/wrap_hits_factory.ts # x-pack/plugins/security_solution/server/lib/detection_engine/signals/wrap_sequences_factory.ts # x-pack/plugins/security_solution/server/plugin.ts # x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts
1 parent 9ac2d2f commit ead4384

File tree

30 files changed

+1015
-234
lines changed

30 files changed

+1015
-234
lines changed

src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ kibana_vars=(
387387
xpack.security.sessionTimeout
388388
xpack.securitySolution.alertMergeStrategy
389389
xpack.securitySolution.alertResultListDefaultDateRange
390+
xpack.securitySolution.alertIgnoreFields
390391
xpack.securitySolution.endpointResultListDefaultFirstPageIndex
391392
xpack.securitySolution.endpointResultListDefaultPageSize
392393
xpack.securitySolution.maxRuleImportExportSize
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import { configSchema } from './config';
9+
10+
describe('config', () => {
11+
describe('alertIgnoreFields', () => {
12+
test('should default to an empty array', () => {
13+
expect(configSchema.validate({}).alertIgnoreFields).toEqual([]);
14+
});
15+
16+
test('should accept an array of strings', () => {
17+
expect(
18+
configSchema.validate({ alertIgnoreFields: ['foo.bar', 'mars.bar'] }).alertIgnoreFields
19+
).toEqual(['foo.bar', 'mars.bar']);
20+
});
21+
22+
test('should throw if a non string is being sent in', () => {
23+
expect(
24+
() =>
25+
configSchema.validate({
26+
alertIgnoreFields: 5,
27+
}).alertIgnoreFields
28+
).toThrow('[alertIgnoreFields]: expected value of type [array] but got [number]');
29+
});
30+
31+
test('should throw if we send in an invalid regular expression as a string', () => {
32+
expect(
33+
() =>
34+
configSchema.validate({
35+
alertIgnoreFields: ['/(/'],
36+
}).alertIgnoreFields
37+
).toThrow(
38+
'[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0'
39+
);
40+
});
41+
42+
test('should throw with two errors if we send two invalid regular expressions', () => {
43+
expect(
44+
() =>
45+
configSchema.validate({
46+
alertIgnoreFields: ['/(/', '/(invalid/'],
47+
}).alertIgnoreFields
48+
).toThrow(
49+
'[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0. "Invalid regular expression: /(invalid/: Unterminated group" at array position 1'
50+
);
51+
});
52+
53+
test('should throw with two errors with a valid string mixed in if we send two invalid regular expressions', () => {
54+
expect(
55+
() =>
56+
configSchema.validate({
57+
alertIgnoreFields: ['/(/', 'valid.string', '/(invalid/'],
58+
}).alertIgnoreFields
59+
).toThrow(
60+
'[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0. "Invalid regular expression: /(invalid/: Unterminated group" at array position 2'
61+
);
62+
});
63+
64+
test('should accept a valid regular expression within the string', () => {
65+
expect(
66+
configSchema.validate({
67+
alertIgnoreFields: ['/(.*)/'],
68+
}).alertIgnoreFields
69+
).toEqual(['/(.*)/']);
70+
});
71+
72+
test('should accept two valid regular expressions', () => {
73+
expect(
74+
configSchema.validate({
75+
alertIgnoreFields: ['/(.*)/', '/(.valid*)/'],
76+
}).alertIgnoreFields
77+
).toEqual(['/(.*)/', '/(.valid*)/']);
78+
});
79+
});
80+
});

x-pack/plugins/security_solution/server/config.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,61 @@ export const configSchema = schema.object({
2121
maxRuleImportPayloadBytes: schema.number({ defaultValue: 10485760 }),
2222
maxTimelineImportExportSize: schema.number({ defaultValue: 10000 }),
2323
maxTimelineImportPayloadBytes: schema.number({ defaultValue: 10485760 }),
24+
25+
/**
26+
* This is used within the merge strategies:
27+
* server/lib/detection_engine/signals/source_fields_merging
28+
*
29+
* For determining which strategy for merging "fields" and "_source" together to get
30+
* runtime fields, constant keywords, etc...
31+
*
32+
* "missingFields" (default) This will only merge fields that are missing from the _source and exist in the fields.
33+
* "noFields" This will turn off all merging of runtime fields, constant keywords from fields.
34+
* "allFields" This will merge and overwrite anything found within "fields" into "_source" before indexing the data.
35+
*/
2436
alertMergeStrategy: schema.oneOf(
2537
[schema.literal('allFields'), schema.literal('missingFields'), schema.literal('noFields')],
2638
{
2739
defaultValue: 'missingFields',
2840
}
2941
),
42+
43+
/**
44+
* This is used within the merge strategies:
45+
* server/lib/detection_engine/signals/source_fields_merging
46+
*
47+
* For determining if we need to ignore particular "fields" and not merge them with "_source" such as
48+
* runtime fields, constant keywords, etc...
49+
*
50+
* This feature and functionality is mostly as "safety feature" meaning that we have had bugs in the past
51+
* where something down the stack unexpectedly ends up in the fields API which causes documents to not
52+
* be indexable. Rather than changing alertMergeStrategy to be "noFields", you can use this array to add
53+
* any problematic values.
54+
*
55+
* You can use plain dotted notation strings such as "host.name" or a regular expression such as "/host\..+/"
56+
*/
57+
alertIgnoreFields: schema.arrayOf(schema.string(), {
58+
defaultValue: [],
59+
validate(ignoreFields) {
60+
const errors = ignoreFields.flatMap((ignoreField, index) => {
61+
if (ignoreField.startsWith('/') && ignoreField.endsWith('/')) {
62+
try {
63+
new RegExp(ignoreField.slice(1, -1));
64+
return [];
65+
} catch (error) {
66+
return [`"${error.message}" at array position ${index}`];
67+
}
68+
} else {
69+
return [];
70+
}
71+
});
72+
if (errors.length !== 0) {
73+
return errors.join('. ');
74+
} else {
75+
return undefined;
76+
}
77+
},
78+
}),
3079
[SIGNALS_INDEX_KEY]: schema.string({ defaultValue: DEFAULT_SIGNALS_INDEX }),
3180

3281
/**

x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export const createMockConfig = (): ConfigType => ({
3131
packagerTaskInterval: '60s',
3232
validateArtifactDownloads: true,
3333
alertMergeStrategy: 'missingFields',
34+
alertIgnoreFields: [],
3435
prebuiltRulesFromFileSystem: true,
3536
prebuiltRulesFromSavedObjects: false,
3637
});

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.test.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ describe('buildBulkBody', () => {
4141
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(
4242
ruleSO,
4343
doc,
44-
'missingFields'
44+
'missingFields',
45+
[]
4546
);
4647
// Timestamp will potentially always be different so remove it for the test
4748
delete fakeSignalSourceHit['@timestamp'];
@@ -109,7 +110,8 @@ describe('buildBulkBody', () => {
109110
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(
110111
ruleSO,
111112
doc,
112-
'missingFields'
113+
'missingFields',
114+
[]
113115
);
114116
// Timestamp will potentially always be different so remove it for the test
115117
delete fakeSignalSourceHit['@timestamp'];
@@ -191,7 +193,8 @@ describe('buildBulkBody', () => {
191193
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(
192194
ruleSO,
193195
doc,
194-
'missingFields'
196+
'missingFields',
197+
[]
195198
);
196199
// Timestamp will potentially always be different so remove it for the test
197200
delete fakeSignalSourceHit['@timestamp'];
@@ -259,7 +262,8 @@ describe('buildBulkBody', () => {
259262
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(
260263
ruleSO,
261264
doc,
262-
'missingFields'
265+
'missingFields',
266+
[]
263267
);
264268
// Timestamp will potentially always be different so remove it for the test
265269
delete fakeSignalSourceHit['@timestamp'];
@@ -324,7 +328,8 @@ describe('buildBulkBody', () => {
324328
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(
325329
ruleSO,
326330
doc,
327-
'missingFields'
331+
'missingFields',
332+
[]
328333
);
329334
// Timestamp will potentially always be different so remove it for the test
330335
delete fakeSignalSourceHit['@timestamp'];
@@ -388,7 +393,8 @@ describe('buildBulkBody', () => {
388393
const { '@timestamp': timestamp, ...fakeSignalSourceHit } = buildBulkBody(
389394
ruleSO,
390395
doc,
391-
'missingFields'
396+
'missingFields',
397+
[]
392398
);
393399
const expected: Omit<SignalHit, '@timestamp'> & { someKey: string } = {
394400
someKey: 'someValue',
@@ -448,7 +454,8 @@ describe('buildBulkBody', () => {
448454
const { '@timestamp': timestamp, ...fakeSignalSourceHit } = buildBulkBody(
449455
ruleSO,
450456
doc,
451-
'missingFields'
457+
'missingFields',
458+
[]
452459
);
453460
const expected: Omit<SignalHit, '@timestamp'> & { someKey: string } = {
454461
someKey: 'someValue',
@@ -677,7 +684,8 @@ describe('buildSignalFromEvent', () => {
677684
ancestor,
678685
ruleSO,
679686
true,
680-
'missingFields'
687+
'missingFields',
688+
[]
681689
);
682690

683691
// Timestamp will potentially always be different so remove it for the test

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_bulk_body.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ import type { ConfigType } from '../../../config';
3535
export const buildBulkBody = (
3636
ruleSO: SavedObject<AlertAttributes>,
3737
doc: SignalSourceHit,
38-
mergeStrategy: ConfigType['alertMergeStrategy']
38+
mergeStrategy: ConfigType['alertMergeStrategy'],
39+
ignoreFields: ConfigType['alertIgnoreFields']
3940
): SignalHit => {
40-
const mergedDoc = getMergeStrategy(mergeStrategy)({ doc });
41+
const mergedDoc = getMergeStrategy(mergeStrategy)({ doc, ignoreFields });
4142
const rule = buildRuleWithOverrides(ruleSO, mergedDoc._source ?? {});
4243
const signal: Signal = {
4344
...buildSignal([mergedDoc], rule),
@@ -68,11 +69,12 @@ export const buildSignalGroupFromSequence = (
6869
sequence: EqlSequence<SignalSource>,
6970
ruleSO: SavedObject<AlertAttributes>,
7071
outputIndex: string,
71-
mergeStrategy: ConfigType['alertMergeStrategy']
72+
mergeStrategy: ConfigType['alertMergeStrategy'],
73+
ignoreFields: ConfigType['alertIgnoreFields']
7274
): WrappedSignalHit[] => {
7375
const wrappedBuildingBlocks = wrapBuildingBlocks(
7476
sequence.events.map((event) => {
75-
const signal = buildSignalFromEvent(event, ruleSO, false, mergeStrategy);
77+
const signal = buildSignalFromEvent(event, ruleSO, false, mergeStrategy, ignoreFields);
7678
signal.signal.rule.building_block_type = 'default';
7779
return signal;
7880
}),
@@ -134,9 +136,10 @@ export const buildSignalFromEvent = (
134136
event: BaseSignalHit,
135137
ruleSO: SavedObject<AlertAttributes>,
136138
applyOverrides: boolean,
137-
mergeStrategy: ConfigType['alertMergeStrategy']
139+
mergeStrategy: ConfigType['alertMergeStrategy'],
140+
ignoreFields: ConfigType['alertIgnoreFields']
138141
): SignalHit => {
139-
const mergedEvent = getMergeStrategy(mergeStrategy)({ doc: event });
142+
const mergedEvent = getMergeStrategy(mergeStrategy)({ doc: event, ignoreFields });
140143
const rule = applyOverrides
141144
? buildRuleWithOverrides(ruleSO, mergedEvent._source ?? {})
142145
: buildRuleWithoutOverrides(ruleSO);

x-pack/plugins/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ describe('searchAfterAndBulkCreate', () => {
7171
ruleSO,
7272
signalsIndex: DEFAULT_SIGNALS_INDEX,
7373
mergeStrategy: 'missingFields',
74+
ignoreFields: [],
7475
});
7576
});
7677

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ describe('signal_rule_alert_type', () => {
194194
ml: mlMock,
195195
lists: listMock.createSetup(),
196196
mergeStrategy: 'missingFields',
197+
ignoreFields: [],
197198
});
198199
});
199200

x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,15 @@ export const signalRulesAlertType = ({
7777
ml,
7878
lists,
7979
mergeStrategy,
80+
ignoreFields,
8081
}: {
8182
logger: Logger;
8283
eventsTelemetry: TelemetryEventsSender | undefined;
8384
version: string;
8485
ml: SetupPlugins['ml'];
8586
lists: SetupPlugins['lists'] | undefined;
8687
mergeStrategy: ConfigType['alertMergeStrategy'];
88+
ignoreFields: ConfigType['alertIgnoreFields'];
8789
}): SignalRuleAlertTypeDefinition => {
8890
return {
8991
id: SIGNALS_ID,
@@ -237,12 +239,14 @@ export const signalRulesAlertType = ({
237239
ruleSO: savedObject,
238240
signalsIndex: params.outputIndex,
239241
mergeStrategy,
242+
ignoreFields,
240243
});
241244

242245
const wrapSequences = wrapSequencesFactory({
243246
ruleSO: savedObject,
244247
signalsIndex: params.outputIndex,
245248
mergeStrategy,
249+
ignoreFields,
246250
});
247251

248252
if (isMlRule(type)) {

0 commit comments

Comments
 (0)