Skip to content

Commit

Permalink
[Security Solutions][Detection Engine] Fixes timestamp bugs within so…
Browse files Browse the repository at this point in the history
…urce indexes when the formats are not ISO8601 format (elastic#101349)

## Summary

We have a few bugs where when the source index for detections is not `"strict_date_optional_time"` it is possible that we will misinterpret the format to be epoch milliseconds when it could be epoch seconds or another ambiguous format or blow up when trying to write out the signals index. This fixes it to where we query for the source index format as an ISO8601 and when we copy the date time format we copy it back out as ISO8601 and insert it into the signal index as ISO8601.

See this [gist](https://gist.github.com/FrankHassanabad/f614ec9762d59cd1129b3269f5bae41c) for more details of how this was accidentally introduced when we added support for runtime fields and the general idea of the fix.

* Removes `docvalue_field` and we now only use `fields` in detection engine search requests
* Splits out the timestamp e2e tests into their own file for `timestamps` file
* Adds more tests to ensure we copy what we expect and we are converting to ISO8601 in the signals
* Removes `ts-expect-error` in a lot of areas including tests and then I fix the types and issues once it is removed. 

### 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 authored and kibanamachine committed Jun 4, 2021
1 parent eed0eec commit 34c35ae
Show file tree
Hide file tree
Showing 27 changed files with 505 additions and 243 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export const sampleDocWithSortId = (
export const sampleDocNoSortId = (
someUuid: string = sampleIdGuid,
ip?: string
): SignalSourceHit => ({
): SignalSourceHit & { _source: Required<SignalSourceHit>['_source'] } => ({
_index: 'myFakeSignalIndex',
_type: 'doc',
_score: 100,
Expand Down Expand Up @@ -225,12 +225,12 @@ export const sampleWrappedSignalHit = (): WrappedSignalHit => {
};
};

export const sampleDocWithAncestors = (): SignalSearchResponse => {
export const sampleDocWithAncestors = (): SignalSearchResponse & {
hits: { hits: Array<ReturnType<typeof sampleDocNoSortId>> };
} => {
const sampleDoc = sampleDocNoSortId();
delete sampleDoc.sort;
// @ts-expect-error @elastic/elasticsearch _source is optional
delete sampleDoc._source.source;
// @ts-expect-error @elastic/elasticsearch _source is optional
sampleDoc._source.signal = {
parent: {
id: 'd5e8eb51-a6a0-456d-8a15-4b79bfec3d71',
Expand Down Expand Up @@ -562,7 +562,9 @@ export const sampleBulkCreateErrorResult = {

export const sampleDocSearchResultsNoSortId = (
someUuid: string = sampleIdGuid
): SignalSearchResponse => ({
): SignalSearchResponse & {
hits: { hits: Array<ReturnType<typeof sampleDocNoSortId>> };
} => ({
took: 10,
timed_out: false,
_shards: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ import { SignalHit, SignalSourceHit } from './types';
import { SIGNALS_TEMPLATE_VERSION } from '../routes/index/get_signals_template';
import { getQueryRuleParams, getThresholdRuleParams } from '../schemas/rule_schemas.mock';

// This allows us to not have to use ts-expect-error with delete in the code.
type SignalHitOptionalTimestamp = Omit<SignalHit, '@timestamp'> & {
'@timestamp'?: SignalHit['@timestamp'];
};

describe('buildBulkBody', () => {
beforeEach(() => {
jest.clearAllMocks();
Expand All @@ -32,11 +37,9 @@ describe('buildBulkBody', () => {
test('bulk body builds well-defined body', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.source;
const fakeSignalSourceHit = buildBulkBody(ruleSO, doc);
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(ruleSO, doc);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete fakeSignalSourceHit['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand Down Expand Up @@ -69,7 +72,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -81,9 +84,8 @@ describe('buildBulkBody', () => {
test('bulk body builds well-defined body with threshold results', () => {
const ruleSO = sampleRuleSO(getThresholdRuleParams());
const baseDoc = sampleDocNoSortId();
const doc: SignalSourceHit = {
const doc: SignalSourceHit & { _source: Required<SignalSourceHit>['_source'] } = {
...baseDoc,
// @ts-expect-error @elastic/elasticsearch _source is optional
_source: {
...baseDoc._source,
threshold_result: {
Expand All @@ -96,11 +98,9 @@ describe('buildBulkBody', () => {
},
},
};
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.source;
const fakeSignalSourceHit = buildBulkBody(ruleSO, doc);
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(ruleSO, doc);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete fakeSignalSourceHit['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand Down Expand Up @@ -133,7 +133,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: {
...expectedRule(),
Expand Down Expand Up @@ -167,18 +167,15 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.source;
// @ts-expect-error @elastic/elasticsearch _source is optional
doc._source.event = {
action: 'socket_opened',
module: 'system',
dataset: 'socket',
kind: 'event',
};
const fakeSignalSourceHit = buildBulkBody(ruleSO, doc);
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(ruleSO, doc);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete fakeSignalSourceHit['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand Down Expand Up @@ -220,7 +217,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -232,17 +229,14 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with but no kind information', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.source;
// @ts-expect-error @elastic/elasticsearch _source is optional
doc._source.event = {
action: 'socket_opened',
module: 'system',
dataset: 'socket',
};
const fakeSignalSourceHit = buildBulkBody(ruleSO, doc);
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(ruleSO, doc);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete fakeSignalSourceHit['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand Down Expand Up @@ -283,7 +277,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -295,15 +289,12 @@ describe('buildBulkBody', () => {
test('bulk body builds original_event if it exists on the event to begin with with only kind information', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.source;
// @ts-expect-error @elastic/elasticsearch _source is optional
doc._source.event = {
kind: 'event',
};
const fakeSignalSourceHit = buildBulkBody(ruleSO, doc);
const fakeSignalSourceHit: SignalHitOptionalTimestamp = buildBulkBody(ruleSO, doc);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete fakeSignalSourceHit['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand Down Expand Up @@ -339,7 +330,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -351,7 +342,6 @@ describe('buildBulkBody', () => {
test('bulk body builds "original_signal" if it exists already as a numeric', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const sampleDoc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete sampleDoc._source.source;
const doc = ({
...sampleDoc,
Expand Down Expand Up @@ -393,7 +383,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -405,7 +395,6 @@ describe('buildBulkBody', () => {
test('bulk body builds "original_signal" if it exists already as an object', () => {
const ruleSO = sampleRuleSO(getQueryRuleParams());
const sampleDoc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete sampleDoc._source.source;
const doc = ({
...sampleDoc,
Expand Down Expand Up @@ -447,7 +436,7 @@ describe('buildBulkBody', () => {
depth: 0,
},
],
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
status: 'open',
rule: expectedRule(),
depth: 1,
Expand All @@ -466,9 +455,8 @@ describe('buildSignalFromSequence', () => {
block2._source.new_key = 'new_key_value';
const blocks = [block1, block2];
const ruleSO = sampleRuleSO(getQueryRuleParams());
const signal = buildSignalFromSequence(blocks, ruleSO);
const signal: SignalHitOptionalTimestamp = buildSignalFromSequence(blocks, ruleSO);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete signal['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { new_key: string } = {
new_key: 'new_key_value',
Expand Down Expand Up @@ -552,9 +540,8 @@ describe('buildSignalFromSequence', () => {
block2._source['@timestamp'] = '2021-05-20T22:28:46+0000';
block2._source.someKey = 'someOtherValue';
const ruleSO = sampleRuleSO(getQueryRuleParams());
const signal = buildSignalFromSequence([block1, block2], ruleSO);
const signal: SignalHitOptionalTimestamp = buildSignalFromSequence([block1, block2], ruleSO);
// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete signal['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> = {
event: {
Expand Down Expand Up @@ -635,12 +622,11 @@ describe('buildSignalFromSequence', () => {
describe('buildSignalFromEvent', () => {
test('builds a basic signal from a single event', () => {
const ancestor = sampleDocWithAncestors().hits.hits[0];
// @ts-expect-error @elastic/elasticsearch _source is optional
delete ancestor._source.source;
const ruleSO = sampleRuleSO(getQueryRuleParams());
const signal = buildSignalFromEvent(ancestor, ruleSO, true);
const signal: SignalHitOptionalTimestamp = buildSignalFromEvent(ancestor, ruleSO, true);

// Timestamp will potentially always be different so remove it for the test
// @ts-expect-error
delete signal['@timestamp'];
const expected: Omit<SignalHit, '@timestamp'> & { someKey: 'someValue' } = {
someKey: 'someValue',
Expand All @@ -651,7 +637,7 @@ describe('buildSignalFromEvent', () => {
_meta: {
version: SIGNALS_TEMPLATE_VERSION,
},
original_time: '2020-04-20T21:27:45+0000',
original_time: '2020-04-20T21:27:45.000Z',
parent: {
id: sampleIdGuid,
rule: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ describe('buildEventTypeSignal', () => {

test('it returns the event appended of kind signal if it does not exist', () => {
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
delete doc._source.event;
const eventType = buildEventTypeSignal(doc);
const expected: object = { kind: 'signal' };
Expand All @@ -25,7 +24,6 @@ describe('buildEventTypeSignal', () => {

test('it returns the event appended of kind signal if it is an empty object', () => {
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
doc._source.event = {};
const eventType = buildEventTypeSignal(doc);
const expected: object = { kind: 'signal' };
Expand All @@ -34,7 +32,6 @@ describe('buildEventTypeSignal', () => {

test('it returns the event with kind signal and other properties if they exist', () => {
const doc = sampleDocNoSortId();
// @ts-expect-error @elastic/elasticsearch _source is optional
doc._source.event = {
action: 'socket_opened',
module: 'system',
Expand Down
Loading

0 comments on commit 34c35ae

Please sign in to comment.