Skip to content

Commit

Permalink
[SIEM][Detection Engine] Removes internal tags when copying signals f…
Browse files Browse the repository at this point in the history
…rom rules (#57880)

## Summary

Removes internal tags when copying tags from rules to the signals

Before where you can see internal tags going in:
<img width="1565" alt="Screen Shot 2020-02-14 at 5 17 36 PM" src="https://user-images.githubusercontent.com/1151048/74578011-6d1f4d00-4f4f-11ea-9057-e89896944af8.png">

After where you can see internal tags not being pushed in:
<img width="1555" alt="Screen Shot 2020-02-14 at 5 22 52 PM" src="https://user-images.githubusercontent.com/1151048/74578023-81fbe080-4f4f-11ea-8f37-86f7e5fe29f5.png">

Notice that these two internal tags should no longer be there:
```
__internal_immutable:false
__internal_rule_id:3b1bcf8a-7826-43b8-833b-6485aa700c41
```

### Checklist

Delete any items that are not applicable to this PR.

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

~~- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)~~

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
  • Loading branch information
FrankHassanabad authored Feb 18, 2020
1 parent 0eb38fc commit 48b2a33
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@
*/

import { sampleDocNoSortId, sampleRule } from './__mocks__/es_results';
import { buildSignal, buildAncestor, buildAncestorsSignal } from './build_signal';
import {
buildSignal,
buildAncestor,
buildAncestorsSignal,
removeInternalTagsFromRule,
} from './build_signal';
import { Signal, Ancestor } from './types';
import { INTERNAL_RULE_ID_KEY, INTERNAL_IMMUTABLE_KEY } from '../../../../common/constants';

describe('buildSignal', () => {
beforeEach(() => {
Expand Down Expand Up @@ -132,6 +138,77 @@ describe('buildSignal', () => {
expect(signal).toEqual(expected);
});

test('it builds a signal as expected with original_event if is present and without internal tags in them', () => {
const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
doc._source.event = {
action: 'socket_opened',
dataset: 'socket',
kind: 'event',
module: 'system',
};
const rule = sampleRule();
rule.tags = [
'some fake tag 1',
'some fake tag 2',
`${INTERNAL_RULE_ID_KEY}:rule-1`,
`${INTERNAL_IMMUTABLE_KEY}:true`,
];
const signal = buildSignal(doc, rule);
const expected: Signal = {
parent: {
rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
type: 'event',
index: 'myFakeSignalIndex',
depth: 1,
},
ancestors: [
{
rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
type: 'event',
index: 'myFakeSignalIndex',
depth: 1,
},
],
original_time: 'someTimeStamp',
original_event: {
action: 'socket_opened',
dataset: 'socket',
kind: 'event',
module: 'system',
},
status: 'open',
rule: {
created_by: 'elastic',
description: 'Detecting root and admin users',
enabled: true,
false_positives: [],
from: 'now-6m',
id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
immutable: false,
index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'],
interval: '5m',
risk_score: 50,
rule_id: 'rule-1',
language: 'kuery',
max_signals: 100,
name: 'Detect Root/Admin Users',
output_index: '.siem-signals',
query: 'user.name: root or user.name: admin',
references: ['http://www.example.com', 'https://ww.example.com'],
severity: 'high',
updated_by: 'elastic',
tags: ['some fake tag 1', 'some fake tag 2'],
to: 'now',
type: 'query',
updated_at: signal.rule.updated_at,
created_at: signal.rule.created_at,
},
};
expect(signal).toEqual(expected);
});

test('it builds a ancestor correctly if the parent does not exist', () => {
const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
doc._source.event = {
Expand Down Expand Up @@ -190,6 +267,51 @@ describe('buildSignal', () => {
expect(signal).toEqual(expected);
});

test('it builds a ancestor correctly if the parent does exist without internal tags in them', () => {
const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
doc._source.event = {
action: 'socket_opened',
dataset: 'socket',
kind: 'event',
module: 'system',
};
doc._source.signal = {
parent: {
rule: '98c0bf9e-4d38-46f4-9a6a-8a820426256b',
id: '730ddf9e-5a00-4f85-9ddf-5878ca511a87',
type: 'event',
index: 'myFakeSignalIndex',
depth: 1,
},
ancestors: [
{
rule: '98c0bf9e-4d38-46f4-9a6a-8a820426256b',
id: '730ddf9e-5a00-4f85-9ddf-5878ca511a87',
type: 'event',
index: 'myFakeSignalIndex',
depth: 1,
},
],
};
const rule = sampleRule();
rule.tags = [
'some fake tag 1',
'some fake tag 2',
`${INTERNAL_RULE_ID_KEY}:rule-1`,
`${INTERNAL_IMMUTABLE_KEY}:true`,
];

const signal = buildAncestor(doc, rule);
const expected: Ancestor = {
rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
type: 'signal',
index: 'myFakeSignalIndex',
depth: 2,
};
expect(signal).toEqual(expected);
});

test('it builds a signal ancestor correctly if the parent does not exist', () => {
const doc = sampleDocNoSortId('d5e8eb51-a6a0-456d-8a15-4b79bfec3d71');
doc._source.event = {
Expand Down Expand Up @@ -258,4 +380,40 @@ describe('buildSignal', () => {
];
expect(signal).toEqual(expected);
});

test('it removes internal tags from a typical rule', () => {
const rule = sampleRule();
rule.tags = [
'some fake tag 1',
'some fake tag 2',
`${INTERNAL_RULE_ID_KEY}:rule-1`,
`${INTERNAL_IMMUTABLE_KEY}:true`,
];
const noInternals = removeInternalTagsFromRule(rule);
expect(noInternals).toEqual(sampleRule());
});

test('it works with an empty array', () => {
const rule = sampleRule();
rule.tags = [];
const noInternals = removeInternalTagsFromRule(rule);
const expected = sampleRule();
expected.tags = [];
expect(noInternals).toEqual(expected);
});

test('it works if tags does not exist', () => {
const rule = sampleRule();
delete rule.tags;
const noInternals = removeInternalTagsFromRule(rule);
const expected = sampleRule();
delete expected.tags;
expect(noInternals).toEqual(expected);
});

test('it works if tags contains normal values and no internal values', () => {
const rule = sampleRule();
const noInternals = removeInternalTagsFromRule(rule);
expect(noInternals).toEqual(rule);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { INTERNAL_IDENTIFIER } from '../../../../common/constants';
import { SignalSourceHit, Signal, Ancestor } from './types';
import { OutputRuleAlertRest } from '../types';

Expand Down Expand Up @@ -45,17 +46,32 @@ export const buildAncestorsSignal = (
};

export const buildSignal = (doc: SignalSourceHit, rule: Partial<OutputRuleAlertRest>): Signal => {
const ruleWithoutInternalTags = removeInternalTagsFromRule(rule);
const parent = buildAncestor(doc, rule);
const ancestors = buildAncestorsSignal(doc, rule);
const signal: Signal = {
parent,
ancestors,
original_time: doc._source['@timestamp'],
status: 'open',
rule,
rule: ruleWithoutInternalTags,
};
if (doc._source.event != null) {
return { ...signal, original_event: doc._source.event };
}
return signal;
};

export const removeInternalTagsFromRule = (
rule: Partial<OutputRuleAlertRest>
): Partial<OutputRuleAlertRest> => {
if (rule.tags == null) {
return rule;
} else {
const ruleWithoutInternalTags: Partial<OutputRuleAlertRest> = {
...rule,
tags: rule.tags.filter(tag => !tag.startsWith(INTERNAL_IDENTIFIER)),
};
return ruleWithoutInternalTags;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export default ({ getService }: FtrProviderContext): void => {
expect(bodyToCompare).to.eql(getSimpleRuleOutputWithoutRuleId());
});

// TODO: This is a valid issue and will be fixed in an upcoming PR and then enabled once that PR is merged
it('should return a 200 ok but have a 409 conflict if we attempt to create the same rule_id twice', async () => {
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`)
Expand Down

0 comments on commit 48b2a33

Please sign in to comment.