Skip to content

Commit

Permalink
[8.11] [Security Solution][Detection Engine] fixes ES|QL ECS multifie…
Browse files Browse the repository at this point in the history
…fields issue (#167769) (#168206)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[Security Solution][Detection Engine] fixes ES|QL ECS multifiefields
issue (#167769)](#167769)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Vitalii
Dmyterko","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-06T10:59:41Z","message":"[Security
Solution][Detection Engine] fixes ES|QL ECS multifiefields issue
(#167769)\n\n## Summary\r\n\r\n- fixes
elastic/security-team#7741 by\r\nreplacing
`ecsMap` from hardcoded `@kbn/rule-registry-plugin` to actual\r\nmapping
for alerts indices from `@kbn/alerts-as-data-utils`\r\n- when converting
ES|QL row table results to object, `null` values\r\nskipped, since its
results consists of all existing mappings in searched\r\nindices, if
fields in query are not filtered\r\n\r\n### Checklist\r\n\r\nDelete any
items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"4ebe45d77ee46c2b502c87aee0f89b73f0d3e40f","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","backport:prev-minor","Team:Detection
Engine","v8.11.0","v8.12.0"],"number":167769,"url":"https://github.com/elastic/kibana/pull/167769","mergeCommit":{"message":"[Security
Solution][Detection Engine] fixes ES|QL ECS multifiefields issue
(#167769)\n\n## Summary\r\n\r\n- fixes
elastic/security-team#7741 by\r\nreplacing
`ecsMap` from hardcoded `@kbn/rule-registry-plugin` to actual\r\nmapping
for alerts indices from `@kbn/alerts-as-data-utils`\r\n- when converting
ES|QL row table results to object, `null` values\r\nskipped, since its
results consists of all existing mappings in searched\r\nindices, if
fields in query are not filtered\r\n\r\n### Checklist\r\n\r\nDelete any
items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"4ebe45d77ee46c2b502c87aee0f89b73f0d3e40f"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/167769","number":167769,"mergeCommit":{"message":"[Security
Solution][Detection Engine] fixes ES|QL ECS multifiefields issue
(#167769)\n\n## Summary\r\n\r\n- fixes
elastic/security-team#7741 by\r\nreplacing
`ecsMap` from hardcoded `@kbn/rule-registry-plugin` to actual\r\nmapping
for alerts indices from `@kbn/alerts-as-data-utils`\r\n- when converting
ES|QL row table results to object, `null` values\r\nskipped, since its
results consists of all existing mappings in searched\r\nindices, if
fields in query are not filtered\r\n\r\n### Checklist\r\n\r\nDelete any
items that are not applicable to this PR.\r\n\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"4ebe45d77ee46c2b502c87aee0f89b73f0d3e40f"}}]}]
BACKPORT-->

Co-authored-by: Vitalii Dmyterko <[email protected]>
  • Loading branch information
kibanamachine and vitaliidm authored Oct 6, 2023
1 parent 8278b71 commit 79224cc
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const esqlExecutor = async ({
});

const esqlSearchDuration = makeFloatString(performance.now() - esqlSignalSearchStart);
result.searchAfterTimes = [esqlSearchDuration];
result.searchAfterTimes.push(esqlSearchDuration);

ruleExecutionLogger.debug(`ES|QL query request took: ${esqlSearchDuration}ms`);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ describe('rowToDocument', () => {
const row = ['abcd', null, '8.8.1', 'packetbeat'];
expect(rowToDocument(columns, row)).toEqual({
_id: 'abcd',
'agent.name': null,
'agent.version': '8.8.1',
'agent.type': 'packetbeat',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ export const rowToDocument = (
row: EsqlResultRow
): Record<string, string | null> => {
return columns.reduce<Record<string, string | null>>((acc, column, i) => {
acc[column.name] = row[i];

// skips nulls, as ES|QL return null for each existing mapping field
if (row[i] !== null) {
acc[column.name] = row[i];
}
return acc;
}, {});
};
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,27 @@ describe('stripNonEcsFields', () => {
]);
});

// https://github.com/elastic/sdh-security-team/issues/736
describe('fields that exists in the alerts mapping but not in local ECS(ruleRegistry) definition', () => {
it('should strip object type "device" field if it is supplied as a keyword', () => {
const { result, removed } = stripNonEcsFields({
device: 'test',
message: 'test message',
});

expect(result).toEqual({
message: 'test message',
});

expect(removed).toEqual([
{
key: 'device',
value: 'test',
},
]);
});
});

describe('array fields', () => {
it('should not strip arrays of objects when an object is expected', () => {
const { result, removed } = stripNonEcsFields({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { ecsFieldMap } from '@kbn/rule-registry-plugin/common/assets/field_maps/ecs_field_map';
import { ecsFieldMap } from '@kbn/alerts-as-data-utils';

import { isPlainObject, cloneDeep, isArray } from 'lodash';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,6 @@ export default ({ getService }: FtrProviderContext) => {
'kibana.space_ids': ['default'],
'kibana.alert.rule.tags': [],
'agent.name': 'test-1',
'agent.type': null,
'agent.version': null,
'host.name': null,
id,
'event.kind': 'signal',
'kibana.alert.original_time': expect.any(String),
Expand Down Expand Up @@ -155,8 +152,6 @@ export default ({ getService }: FtrProviderContext) => {
'kibana.alert.workflow_tags': [],
'kibana.alert.rule.risk_score': 55,
'kibana.alert.rule.severity': 'high',
'kibana.alert.original_event.created': null,
'kibana.alert.original_event.ingested': null,
});
});

Expand Down Expand Up @@ -829,5 +824,163 @@ export default ({ getService }: FtrProviderContext) => {
expect(previewAlerts[0]._source).toHaveProperty('host.risk.calculated_score_norm', 1);
});
});

describe('ECS fields validation', () => {
it('creates alert if ECS field has multifields', async () => {
const id = uuidv4();
const interval: [string, string] = ['2020-10-28T06:00:00.000Z', '2020-10-28T06:10:00.000Z'];
const doc1 = { agent: { name: 'test-1' }, 'observer.os.full': 'full test os' };
const doc2 = { agent: { name: 'test-2' } };

const rule: EsqlRuleCreateProps = {
...getCreateEsqlRulesSchemaMock('rule-1', true),
query: `from ecs_compliant [metadata _id] ${internalIdPipe(
id
)} | where agent.name=="test-1"`,
from: 'now-1h',
interval: '1h',
};

await indexEnhancedDocuments({
documents: [doc1, doc2],
interval,
id,
});

const { previewId } = await previewRule({
supertest,
rule,
timeframeEnd: new Date('2020-10-28T06:30:00.000Z'),
});

const previewAlerts = await getPreviewAlerts({
es,
previewId,
size: 10,
});

expect(previewAlerts.length).toBe(1);
expect(previewAlerts[0]._source).toHaveProperty(['observer.os.full'], 'full test os');
// *.text is multifield define in mappings for observer.os.full
expect(previewAlerts[0]._source).not.toHaveProperty(['observer.os.full.text']);
});
// https://github.com/elastic/security-team/issues/7741
it('creates alert if ECS field has multifields and is not in ruleRegistry ECS map', async () => {
const id = uuidv4();
const interval: [string, string] = ['2020-10-28T06:00:00.000Z', '2020-10-28T06:10:00.000Z'];
const doc1 = {
agent: { name: 'test-1' },
// this field is mapped in alerts index, but not in ruleRegistry ECS map
'process.entry_leader.name': 'test_process_name',
};
const doc2 = { agent: { name: 'test-2' } };

const rule: EsqlRuleCreateProps = {
...getCreateEsqlRulesSchemaMock('rule-1', true),
query: `from ecs_compliant [metadata _id] ${internalIdPipe(
id
)} | where agent.name=="test-1"`,
from: 'now-1h',
interval: '1h',
};

await indexEnhancedDocuments({
documents: [doc1, doc2],
interval,
id,
});

const { previewId } = await previewRule({
supertest,
rule,
timeframeEnd: new Date('2020-10-28T06:30:00.000Z'),
});

const previewAlerts = await getPreviewAlerts({
es,
previewId,
size: 10,
});

expect(previewAlerts.length).toBe(1);
expect(previewAlerts[0]._source).toHaveProperty(
['process.entry_leader.name'],
'test_process_name'
);
expect(previewAlerts[0]._source).not.toHaveProperty(['process.entry_leader.name.text']);
expect(previewAlerts[0]._source).not.toHaveProperty(['process.entry_leader.name.caseless']);
});

describe('non-ecs', () => {
before(async () => {
await esArchiver.load(
'x-pack/test/functional/es_archives/security_solution/ecs_non_compliant'
);
});

after(async () => {
await esArchiver.unload(
'x-pack/test/functional/es_archives/security_solution/ecs_non_compliant'
);
});

const { indexEnhancedDocuments: indexEnhancedDocumentsToNonEcs } = dataGeneratorFactory({
es,
index: 'ecs_non_compliant',
log,
});

it('creates alert if non ECS field has multifields', async () => {
const id = uuidv4();
const interval: [string, string] = [
'2020-10-28T06:00:00.000Z',
'2020-10-28T06:10:00.000Z',
];
const doc1 = {
'random.entry_leader.name': 'random non-ecs field',
};

const rule: EsqlRuleCreateProps = {
...getCreateEsqlRulesSchemaMock('rule-1', true),
query: `from ecs_non_compliant [metadata _id] ${internalIdPipe(id)}`,
from: 'now-1h',
interval: '1h',
};

await indexEnhancedDocumentsToNonEcs({
documents: [doc1],
interval,
id,
});

const { previewId } = await previewRule({
supertest,
rule,
timeframeEnd: new Date('2020-10-28T06:30:00.000Z'),
});

const previewAlerts = await getPreviewAlerts({
es,
previewId,
size: 10,
});

expect(previewAlerts.length).toBe(1);
// all multifields have been indexed, which is expected, seen we don't know original mappings
expect(previewAlerts[0]._source).toHaveProperty(
['random.entry_leader.name'],
'random non-ecs field'
);
expect(previewAlerts[0]._source).toHaveProperty(
['random.entry_leader.name.text'],
'random non-ecs field'
);
expect(previewAlerts[0]._source).toHaveProperty(
['random.entry_leader.name.caseless'],
'random non-ecs field'
);
});
});
});
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,45 @@
}
}
},
"observer": {
"properties": {
"os": {
"properties": {
"full": {
"type": "keyword",
"ignore_above": 1024,
"fields": {
"text": {
"type": "match_only_text"
}
}
}
}
}
}
},
"process": {
"properties": {
"entry_leader": {
"properties": {
"name": {
"type": "keyword",
"ignore_above": 1024,
"fields": {
"text": {
"type": "match_only_text"
},
"caseless": {
"type": "keyword",
"ignore_above": 1024,
"normalizer": "lowercase"
}
}
}
}
}
}
},
"host": {
"properties": {
"name": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,28 @@
"type": "text"
}
}
},
"random": {
"properties": {
"entry_leader": {
"properties": {
"name": {
"type": "keyword",
"ignore_above": 1024,
"fields": {
"text": {
"type": "match_only_text"
},
"caseless": {
"type": "keyword",
"ignore_above": 1024,
"normalizer": "lowercase"
}
}
}
}
}
}
}
}
},
Expand Down

0 comments on commit 79224cc

Please sign in to comment.