diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.test.ts index 74445d4be790a..25583ae8aca05 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.test.ts @@ -1327,6 +1327,35 @@ describe('merge_all_fields_with_source', () => { }); }); + test('does not add multi field values such as "process.command_line.text" to nested source when "process.command_line" has value', () => { + const _source: SignalSourceHit['_source'] = { + process: { + command_line: 'string longer than 10 characters', + }, + }; + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeAllFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); + + test('does not add multi field values such as "process.command_line.text" to nested source when "process.command_line" has array value', () => { + const _source: SignalSourceHit['_source'] = { + process: { + command_line: ['string longer than 10 characters'], + }, + }; + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeAllFieldsWithSource({ doc, ignoreFields: [] })._source; + + expect(merged).toEqual(_source); + }); + test('multi-field values mixed with regular values will not be merged accidentally"', () => { const _source: SignalSourceHit['_source'] = {}; const fields: SignalSourceHit['fields'] = { @@ -1393,6 +1422,32 @@ describe('merge_all_fields_with_source', () => { foo: 'other_value_1', }); }); + + test('does not add multi field values such as "process.command_line.text" to flattened source when "process.command_line" has value', () => { + const _source: SignalSourceHit['_source'] = { + 'process.command_line': 'string longer than 10 characters', + }; + + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeAllFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); + + test('does not add multi field values such as "process.command_line.text" to flattened source when "process.command_line" has array value', () => { + const _source: SignalSourceHit['_source'] = { + 'process.command_line': ['string longer than 10 characters'], + }; + + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeAllFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.ts index e3c7f8f5ee50e..bd88fb6be38e1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_all_fields_with_source.ts @@ -15,8 +15,8 @@ import { isNestedObject } from '../utils/is_nested_object'; import { recursiveUnboxingFields } from '../utils/recursive_unboxing_fields'; import { isPrimitive } from '../utils/is_primitive'; import { isArrayOfPrimitives } from '../utils/is_array_of_primitives'; -import { arrayInPathExists } from '../utils/array_in_path_exists'; import { isTypeObject } from '../utils/is_type_object'; +import { isPathValid } from '../utils/is_path_valid'; /** * Merges all of "doc._source" with its "doc.fields" on a "best effort" basis. See ../README.md for more information @@ -107,7 +107,7 @@ const hasEarlyReturnConditions = ({ const valueInMergedDocument = get(fieldsKey, merged); return ( fieldsValue.length === 0 || - (valueInMergedDocument === undefined && arrayInPathExists(fieldsKey, merged)) || + (valueInMergedDocument === undefined && !isPathValid(fieldsKey, merged)) || (isObjectLikeOrArrayOfObjectLikes(valueInMergedDocument) && !isNestedObject(fieldsValue) && !isTypeObject(fieldsValue)) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.test.ts index f5863533ea283..8b794f5304cf8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.test.ts @@ -1283,6 +1283,38 @@ describe('merge_missing_fields_with_source', () => { foo: 'other_value_1', }); }); + + test('does not add multi field values such as "process.command_line.text" to nested source when "process.command_line" has value', () => { + const _source: SignalSourceHit['_source'] = { + '@timestamp': '2023-02-10T10:15:50Z', + process: { + command_line: 'string longer than 10 characters', + }, + }; + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + '@timestamp': ['2023-02-10T10:15:50.000Z'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeMissingFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); + + test('does not add multi field values such as "process.command_line.text" to nested source when "process.command_line" has array value', () => { + const _source: SignalSourceHit['_source'] = { + '@timestamp': '2023-02-10T10:15:50Z', + process: { + command_line: ['string longer than 10 characters'], + }, + }; + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + '@timestamp': ['2023-02-10T10:15:50.000Z'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeMissingFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); }); describe('flattened keys for the _source', () => { @@ -1331,6 +1363,36 @@ describe('merge_missing_fields_with_source', () => { foo: 'other_value_1', }); }); + + test('does not add multi field values such as "process.command_line.text" to flattened source when "process.command_line" has value', () => { + const _source: SignalSourceHit['_source'] = { + '@timestamp': '2023-02-10T10:15:50Z', + 'process.command_line': 'string longer than 10 characters', + }; + + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + '@timestamp': ['2023-02-10T10:15:50.000Z'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeMissingFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); + + test('does not add multi field values such as "process.command_line.text" to flattened source when "process.command_line" has array value', () => { + const _source: SignalSourceHit['_source'] = { + '@timestamp': '2023-02-10T10:15:50Z', + 'process.command_line': ['string longer than 10 characters'], + }; + + const fields: SignalSourceHit['fields'] = { + 'process.command_line.text': ['string longer than 10 characters'], + '@timestamp': ['2023-02-10T10:15:50.000Z'], + }; + const doc: SignalSourceHit = { ...emptyEsResult(), _source, fields }; + const merged = mergeMissingFieldsWithSource({ doc, ignoreFields: [] })._source; + expect(merged).toEqual(_source); + }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.ts index c20f6b55301bd..997fbc920431c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/strategies/merge_missing_fields_with_source.ts @@ -12,8 +12,8 @@ import { filterFieldEntries } from '../utils/filter_field_entries'; import type { FieldsType, MergeStrategyFunction } from '../types'; import { recursiveUnboxingFields } from '../utils/recursive_unboxing_fields'; import { isTypeObject } from '../utils/is_type_object'; -import { arrayInPathExists } from '../utils/array_in_path_exists'; import { isNestedObject } from '../utils/is_nested_object'; +import { isPathValid } from '../utils/is_path_valid'; /** * Merges only missing sections of "doc._source" with its "doc.fields" on a "best effort" basis. See ../README.md for more information @@ -79,7 +79,7 @@ const hasEarlyReturnConditions = ({ return ( fieldsValue.length === 0 || valueInMergedDocument !== undefined || - arrayInPathExists(fieldsKey, merged) || + !isPathValid(fieldsKey, merged) || isNestedObject(fieldsValue) || isTypeObject(fieldsValue) ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.test.ts new file mode 100644 index 0000000000000..e899142bb7352 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.test.ts @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { isPathValid } from './is_path_valid'; + +describe('isPathValid', () => { + test('not valid when empty string and empty object', () => { + expect(isPathValid('', {})).toEqual(false); + }); + + test('valid when a path and empty object', () => { + expect(isPathValid('a.b.c', {})).toEqual(true); + }); + + test('not valid when a path and an array exists', () => { + expect(isPathValid('a', { a: [] })).toEqual(false); + }); + + test('not valid when a path and primitive value exists', () => { + expect(isPathValid('a', { a: 'test' })).toEqual(false); + expect(isPathValid('a', { a: 1 })).toEqual(false); + expect(isPathValid('a', { a: true })).toEqual(false); + }); + + test('valid when a path and object value exists', () => { + expect(isPathValid('a', { a: {} })).toEqual(true); + }); + + test('not valid when a path and an array exists within the parent path at level 1', () => { + expect(isPathValid('a.b', { a: [] })).toEqual(false); + }); + + test('not valid when a path and primitive value exists within the parent path at level 1', () => { + expect(isPathValid('a.b', { a: 'test' })).toEqual(false); + expect(isPathValid('a.b', { a: 1 })).toEqual(false); + expect(isPathValid('a.b', { a: true })).toEqual(false); + }); + + test('valid when a path and object value exists within the parent path at level 1', () => { + expect(isPathValid('a.b', { a: {} })).toEqual(true); + }); + + test('not valid when a path and an array exists within the parent path at level 2', () => { + expect(isPathValid('a.b.c', { a: { b: [] } })).toEqual(false); + }); + + test('not valid when a path and primitive value exists within the parent path at level 2', () => { + expect(isPathValid('a.b', { a: { b: 'test' } })).toEqual(false); + expect(isPathValid('a.b', { a: { b: 1 } })).toEqual(false); + expect(isPathValid('a.b', { a: { b: true } })).toEqual(false); + }); + + test('valid when a path and object value exists within the parent path at level 2', () => { + expect(isPathValid('a.b', { a: { b: {} } })).toEqual(true); + }); + + test('not valid when a path and an array exists within the parent path at level 3', () => { + expect(isPathValid('a.b.c', { a: { b: { c: [] } } })).toEqual(false); + }); + + test('not valid when a path and primitive value exists within the parent path at level 3', () => { + expect(isPathValid('a.b.c', { a: { b: { c: 'test' } } })).toEqual(false); + expect(isPathValid('a.b.c', { a: { b: { c: 1 } } })).toEqual(false); + expect(isPathValid('a.b.c', { a: { b: { c: true } } })).toEqual(false); + }); + + test('valid when a path and object value exists within the parent path at level 3', () => { + expect(isPathValid('a.b.c', { a: { b: { c: {} } } })).toEqual(true); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.ts new file mode 100644 index 0000000000000..a54094100ed18 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/source_fields_merging/utils/is_path_valid.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { get, isPlainObject } from 'lodash/fp'; +import type { SignalSource } from '../../types'; + +/** + * Returns true if path in SignalSource object is valid + * Path is valid if each field in hierarchy is object or undefined + * Path is not valid if ANY of field in hierarchy is not object or undefined + * @param path in source to check within source + * @param source The source document + * @returns boolean + */ +export const isPathValid = (path: string, source: SignalSource): boolean => { + if (!path) { + return false; + } + const splitPath = path.split('.'); + + return splitPath.every((_, index, array) => { + const newPath = [...array].splice(0, index + 1).join('.'); + const valueToCheck = get(newPath, source); + return valueToCheck === undefined || isPlainObject(valueToCheck); + }); +}; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/non_ecs_fields.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/non_ecs_fields.ts index 3c5368b7a23ad..0bcb05b5e1c40 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/non_ecs_fields.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/non_ecs_fields.ts @@ -323,5 +323,63 @@ export default ({ getService }: FtrProviderContext) => { // invalid ECS field is getting removed expect(alertSource).not.toHaveProperty('dll.code_signature.valid'); }); + + describe('multi-fields', () => { + it('should not add multi field .text to ecs compliant nested source', async () => { + const document = { + process: { + command_line: 'string longer than 10 characters', + }, + }; + + const { errors, alertSource } = await indexAndCreatePreviewAlert(document); + + expect(errors).toEqual([]); + + expect(alertSource).toHaveProperty('process', document.process); + expect(alertSource).not.toHaveProperty('process.command_line.text'); + }); + + it('should not add multi field .text to ecs compliant flattened source', async () => { + const document = { + 'process.command_line': 'string longer than 10 characters', + }; + + const { errors, alertSource } = await indexAndCreatePreviewAlert(document); + + expect(errors).toEqual([]); + + expect(alertSource?.['process.command_line']).toEqual(document['process.command_line']); + expect(alertSource).not.toHaveProperty('process.command_line.text'); + }); + + it('should not add multi field .text to ecs non compliant nested source', async () => { + const document = { + nonEcs: { + command_line: 'string longer than 10 characters', + }, + }; + + const { errors, alertSource } = await indexAndCreatePreviewAlert(document); + + expect(errors).toEqual([]); + + expect(alertSource).toHaveProperty('nonEcs', document.nonEcs); + expect(alertSource).not.toHaveProperty('nonEcs.command_line.text'); + }); + + it('should not add multi field .text to ecs non compliant flattened source', async () => { + const document = { + 'nonEcs.command_line': 'string longer than 10 characters', + }; + + const { errors, alertSource } = await indexAndCreatePreviewAlert(document); + + expect(errors).toEqual([]); + + expect(alertSource?.['nonEcs.command_line']).toEqual(document['nonEcs.command_line']); + expect(alertSource).not.toHaveProperty('nonEcs.command_line.text'); + }); + }); }); }; diff --git a/x-pack/test/functional/es_archives/security_solution/ecs_non_compliant/mappings.json b/x-pack/test/functional/es_archives/security_solution/ecs_non_compliant/mappings.json index 42d23e794ba23..40408d65b6d89 100644 --- a/x-pack/test/functional/es_archives/security_solution/ecs_non_compliant/mappings.json +++ b/x-pack/test/functional/es_archives/security_solution/ecs_non_compliant/mappings.json @@ -55,6 +55,24 @@ } } } + }, + "process.command_line": { + "type": "keyword", + "ignore_above": 10, + "fields": { + "text": { + "type": "text" + } + } + }, + "nonEcs.command_line": { + "type": "keyword", + "ignore_above": 10, + "fields": { + "text": { + "type": "text" + } + } } } },