Skip to content

Commit

Permalink
[SEIM][Detection Engine] Time gap detection and logging
Browse files Browse the repository at this point in the history 
## Summary

This adds utilities and logging of time gap detection. Gaps happen whenever rules begin to fall behind their interval. This isn't a perfect works for all inputs and if it detects unexpected input that is not of an interval format (but could be valid date time math) it will just return null and ignore it.

This also fixes a bug with interval where we were using the object instead of the primitive since alerting team changed their structure.

For testing, fire up any rule and shutdown Kibana for more than 6 minutes and then when restarting you should see the warning message. 



### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

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

~~- [ ] 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)~~

### 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)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
  • Loading branch information
@FrankHassanabad
FrankHassanabad authored Jan 13, 2020
1 parent ebd2c21 commit 641c670
Show file tree
Hide file tree
Showing 3 changed files with 341 additions and 5 deletions.
22 changes: 17 additions & 5 deletions x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { schema } from '@kbn/config-schema';
import { Logger } from 'src/core/server';
import moment from 'moment';
import {
SIGNALS_ID,
DEFAULT_MAX_SIGNALS,
Expand All @@ -17,6 +18,7 @@ import { getInputIndex } from './get_input_output_index';
import { searchAfterAndBulkCreate } from './search_after_bulk_create';
import { getFilter } from './get_filter';
import { SignalRuleAlertTypeDefinition } from './types';
import { getGapBetweenRuns } from './utils';

export const signalRulesAlertType = ({
logger,
Expand Down Expand Up @@ -57,7 +59,8 @@ export const signalRulesAlertType = ({
version: schema.number({ defaultValue: 1 }),
}),
},
async executor({ alertId, services, params }) {
// fun fact: previousStartedAt is not actually a Date but a String of a date
async executor({ previousStartedAt, alertId, services, params }) {
const {
from,
ruleId,
Expand All @@ -70,17 +73,26 @@ export const signalRulesAlertType = ({
to,
type,
} = params;

// TODO: Remove this hard extraction of name once this is fixed: https://github.com/elastic/kibana/issues/50522
const savedObject = await services.savedObjectsClient.get('alert', alertId);
const name: string = savedObject.attributes.name;
const tags: string[] = savedObject.attributes.tags;

const createdBy: string = savedObject.attributes.createdBy;
const updatedBy: string = savedObject.attributes.updatedBy;
const interval: string = savedObject.attributes.interval;
const interval: string = savedObject.attributes.schedule.interval;
const enabled: boolean = savedObject.attributes.enabled;

const gap = getGapBetweenRuns({
previousStartedAt: previousStartedAt != null ? moment(previousStartedAt) : null, // TODO: Remove this once previousStartedAt is no longer a string
interval,
from,
to,
});
if (gap != null && gap.asMilliseconds() > 0) {
logger.warn(
`Signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}" has a time gap of ${gap.humanize()} (${gap.asMilliseconds()}ms), and could be missing signals within that time. Consider increasing your look behind time or adding more Kibana instances.`
);
}
// set searchAfter page size to be the lesser of default page size or maxSignals.
const searchAfterSize =
DEFAULT_SEARCH_AFTER_PAGE_SIZE <= params.maxSignals
Expand Down Expand Up @@ -155,7 +167,7 @@ export const signalRulesAlertType = ({
// TODO: Error handling and writing of errors into a signal that has error
// handling/conditions
logger.error(
`Error from signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}"`
`Error from signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}" message: ${err.message}`
);
}
},
Expand Down
258 changes: 258 additions & 0 deletions x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/utils.test.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import moment from 'moment';

import { generateId, parseInterval, getDriftTolerance, getGapBetweenRuns } from './utils';

describe('utils', () => {
let nowDate = moment('2020-01-01T00:00:00.000Z');

beforeEach(() => {
nowDate = moment('2020-01-01T00:00:00.000Z');
});

describe('generateId', () => {
test('it generates expected output', () => {
const id = generateId('index-123', 'doc-123', 'version-123', 'rule-123');
expect(id).toEqual('10622e7d06c9e38a532e71fc90e3426c1100001fb617aec8cb974075da52db06');
});

test('expected output is a hex', () => {
const id = generateId('index-123', 'doc-123', 'version-123', 'rule-123');
expect(id).toMatch(/[a-f0-9]+/);
});
});

describe('getIntervalMilliseconds', () => {
test('it returns a duration when given one that is valid', () => {
const duration = parseInterval('5m');
expect(duration).not.toBeNull();
expect(duration?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds());
});

test('it returns null given an invalid duration', () => {
const duration = parseInterval('junk');
expect(duration).toBeNull();
});
});

describe('getDriftToleranceMilliseconds', () => {
test('it returns a drift tolerance in milliseconds of 1 minute when from overlaps to by 1 minute and the interval is 5 minutes', () => {
const drift = getDriftTolerance({
from: 'now-6m',
to: 'now',
interval: moment.duration(5, 'minutes'),
});
expect(drift).not.toBeNull();
expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds());
});

test('it returns a drift tolerance of 0 when from equals the interval', () => {
const drift = getDriftTolerance({
from: 'now-5m',
to: 'now',
interval: moment.duration(5, 'minutes'),
});
expect(drift?.asMilliseconds()).toEqual(0);
});

test('it returns a drift tolerance of 5 minutes when from is 10 minutes but the interval is 5 minutes', () => {
const drift = getDriftTolerance({
from: 'now-10m',
to: 'now',
interval: moment.duration(5, 'minutes'),
});
expect(drift).not.toBeNull();
expect(drift?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds());
});

test('it returns a drift tolerance of 10 minutes when from is 10 minutes ago and the interval is 0', () => {
const drift = getDriftTolerance({
from: 'now-10m',
to: 'now',
interval: moment.duration(0, 'milliseconds'),
});
expect(drift).not.toBeNull();
expect(drift?.asMilliseconds()).toEqual(moment.duration(10, 'minutes').asMilliseconds());
});

test('returns null if the "to" is not "now" since we have limited support for date math', () => {
const drift = getDriftTolerance({
from: 'now-6m',
to: 'invalid', // if not set to "now" this function returns null
interval: moment.duration(1000, 'milliseconds'),
});
expect(drift).toBeNull();
});

test('returns null if the "from" does not start with "now-" since we have limited support for date math', () => {
const drift = getDriftTolerance({
from: 'valid', // if not set to "now-x" where x is an interval such as 6m
to: 'now',
interval: moment.duration(1000, 'milliseconds'),
});
expect(drift).toBeNull();
});

test('returns null if the "from" starts with "now-" but has a string instead of an integer', () => {
const drift = getDriftTolerance({
from: 'now-dfdf', // if not set to "now-x" where x is an interval such as 6m
to: 'now',
interval: moment.duration(1000, 'milliseconds'),
});
expect(drift).toBeNull();
});
});

describe('getGapBetweenRuns', () => {
test('it returns a gap of 0 when from and interval match each other and the previous started was from the previous interval time', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
interval: '5m',
from: 'now-5m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(0);
});

test('it returns a negative gap of 1 minute when from overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
interval: '5m',
from: 'now-6m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(-1, 'minute').asMilliseconds());
});

test('it returns a negative gap of 5 minutes when from overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
interval: '5m',
from: 'now-10m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(-5, 'minute').asMilliseconds());
});

test('it returns a negative gap of 1 minute when from overlaps to by 1 minute and the previousStartedAt was 10 minutes ago and so was the interval', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(10, 'minutes'),
interval: '10m',
from: 'now-11m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(-1, 'minute').asMilliseconds());
});

test('it returns a gap of only -30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is 30 seconds more', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate
.clone()
.subtract(5, 'minutes')
.subtract(30, 'seconds'),
interval: '5m',
from: 'now-6m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(-30, 'seconds').asMilliseconds());
});

test('it returns an exact 0 gap when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute late', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(6, 'minutes'),
interval: '5m',
from: 'now-6m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(0, 'minute').asMilliseconds());
});

test('it returns a gap of 30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute and 30 seconds late', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate
.clone()
.subtract(6, 'minutes')
.subtract(30, 'seconds'),
interval: '5m',
from: 'now-6m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(30, 'seconds').asMilliseconds());
});

test('it returns a gap of 1 minute when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is two minutes late', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone().subtract(7, 'minutes'),
interval: '5m',
from: 'now-6m',
to: 'now',
now: nowDate.clone(),
});
expect(gap?.asMilliseconds()).not.toBeNull();
expect(gap?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds());
});

test('it returns null if given a previousStartedAt of null', () => {
const gap = getGapBetweenRuns({
previousStartedAt: null,
interval: '5m',
from: 'now-5m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).toBeNull();
});

test('it returns null if the interval is an invalid string such as "invalid"', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone(),
interval: 'invalid', // if not set to "x" where x is an interval such as 6m
from: 'now-5m',
to: 'now',
now: nowDate.clone(),
});
expect(gap).toBeNull();
});

test('it returns null if from is an invalid string such as "invalid"', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone(),
interval: '5m',
from: 'invalid', // if not set to "now-x" where x is an interval such as 6m
to: 'now',
now: nowDate.clone(),
});
expect(gap).toBeNull();
});

test('it returns null if to is an invalid string such as "invalid"', () => {
const gap = getGapBetweenRuns({
previousStartedAt: nowDate.clone(),
interval: '5m',
from: 'now-5m',
to: 'invalid', // if not set to "now" this function returns null
now: nowDate.clone(),
});
expect(gap).toBeNull();
});
});
});
Loading

0 comments on commit 641c670

Please sign in to comment.