[Security Solution] [Detections] Updates rules routes to validate "from" param on rules (#76000)

* updates validation on 'from' param to prevent malformed datemath strings from being accepted

* fix imports

* copy paste is not my friend

* missed type check somehow

* forgot to mock common utils

* updates bodies for request validation tests
This commit is contained in:
Devin W. Hurley 2020-08-26 18:18:39 -04:00 committed by GitHub
parent 595dfdb023
commit 979d1dbca8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 246 additions and 30 deletions

View file

@ -7,11 +7,14 @@
/* eslint-disable @typescript-eslint/naming-convention */
import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { RiskScore } from '../types/risk_score';
import { UUID } from '../types/uuid';
import { IsoDateString } from '../types/iso_date_string';
import { PositiveIntegerGreaterThanZero } from '../types/positive_integer_greater_than_zero';
import { PositiveInteger } from '../types/positive_integer';
import { parseScheduleDates } from '../../utils';
export const author = t.array(t.string);
export type Author = t.TypeOf<typeof author>;
@ -76,8 +79,18 @@ export const action = t.exact(
export const actions = t.array(action);
export type Actions = t.TypeOf<typeof actions>;
// TODO: Create a regular expression type or custom date math part type here
export const from = t.string;
const stringValidator = (input: unknown): input is string => typeof input === 'string';
export const from = new t.Type<string, string, unknown>(
'From',
t.string.is,
(input, context): Either<t.Errors, string> => {
if (stringValidator(input) && parseScheduleDates(input) == null) {
return t.failure(input, context, 'Failed to parse "from" on rule param');
}
return t.string.validate(input, context);
},
t.identity
);
export type From = t.TypeOf<typeof from>;
export const fromOrUndefined = t.union([from, t.undefined]);

View file

@ -6,7 +6,7 @@
import * as t from 'io-ts';
import { Either } from 'fp-ts/lib/Either';
import { from } from '../common/schemas';
/**
* Types the DefaultFromString as:
* - If null or undefined, then a default of the string "now-6m" will be used
@ -14,7 +14,11 @@ import { Either } from 'fp-ts/lib/Either';
export const DefaultFromString = new t.Type<string, string | undefined, unknown>(
'DefaultFromString',
t.string.is,
(input, context): Either<t.Errors, string> =>
input == null ? t.success('now-6m') : t.string.validate(input, context),
(input, context): Either<t.Errors, string> => {
if (input == null) {
return t.success('now-6m');
}
return from.validate(input, context);
},
t.identity
);

View file

@ -4,6 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/
import moment from 'moment';
import dateMath from '@elastic/datemath';
import { EntriesArray } from '../shared_imports';
import { RuleType } from './types';
@ -18,3 +21,15 @@ export const hasNestedEntry = (entries: EntriesArray): boolean => {
};
export const isThresholdRule = (ruleType: RuleType) => ruleType === 'threshold';
export const parseScheduleDates = (time: string): moment.Moment | null => {
const isValidDateString = !isNaN(Date.parse(time));
const isValidInput = isValidDateString || time.trim().startsWith('now');
const formattedDate = isValidDateString
? moment(time)
: isValidInput
? dateMath.parse(time)
: null;
return formattedDate ?? null;
};

View file

@ -14,7 +14,7 @@ import { RuleAlertAttributes } from '../signals/types';
import { siemRuleActionGroups } from '../signals/siem_rule_action_groups';
import { scheduleNotificationActions } from './schedule_notification_actions';
import { getNotificationResultsLink } from './utils';
import { parseScheduleDates } from '../signals/utils';
import { parseScheduleDates } from '../../../../common/detection_engine/utils';
export const rulesNotificationAlertType = ({
logger,

View file

@ -161,6 +161,17 @@ describe('create_rules_bulk', () => {
expect(result.ok).toHaveBeenCalled();
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'post',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_create`,
body: [{ from: 'now-7m', interval: '5m', ...getCreateRulesSchemaMock() }],
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows unknown rule type', async () => {
const request = requestMock.create({
method: 'post',
@ -173,5 +184,21 @@ describe('create_rules_bulk', () => {
'Invalid value "unexpected_type" supplied to "type"'
);
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'post',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_create`,
body: [
{
from: 'now-3755555555555555.67s',
interval: '5m',
...getCreateRulesSchemaMock(),
},
],
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -164,5 +164,30 @@ describe('create_rules', () => {
'Invalid value "unexpected_type" supplied to "type"'
);
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'post',
path: DETECTION_ENGINE_RULES_URL,
body: { from: 'now-7m', interval: '5m', ...getCreateRulesSchemaMock() },
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'post',
path: DETECTION_ENGINE_RULES_URL,
body: {
from: 'now-3755555555555555.67s',
interval: '5m',
...getCreateRulesSchemaMock(),
},
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -183,5 +183,32 @@ describe('patch_rules_bulk', () => {
'Invalid value "unknown_type" supplied to "type"'
);
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'patch',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
body: [{ from: 'now-7m', interval: '5m', ...getCreateRulesSchemaMock() }],
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'patch',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
body: [
{
from: 'now-3755555555555555.67s',
interval: '5m',
...getCreateRulesSchemaMock(),
},
],
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -18,7 +18,7 @@ import {
} from '../__mocks__/request_responses';
import { requestContextMock, serverMock, requestMock } from '../__mocks__';
import { patchRulesRoute } from './patch_rules_route';
import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/create_rules_schema.mock';
import { getPatchRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/patch_rules_schema.mock';
jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
@ -156,7 +156,7 @@ describe('patch_rules', () => {
const request = requestMock.create({
method: 'patch',
path: DETECTION_ENGINE_RULES_URL,
body: { ...getCreateRulesSchemaMock(), rule_id: undefined },
body: { ...getPatchRulesSchemaMock(), rule_id: undefined },
});
const response = await server.inject(request, context);
expect(response.body).toEqual({
@ -169,7 +169,7 @@ describe('patch_rules', () => {
const request = requestMock.create({
method: 'patch',
path: DETECTION_ENGINE_RULES_URL,
body: { ...getCreateRulesSchemaMock(), type: 'query' },
body: { ...getPatchRulesSchemaMock(), type: 'query' },
});
const result = server.validate(request);
@ -180,7 +180,7 @@ describe('patch_rules', () => {
const request = requestMock.create({
method: 'patch',
path: DETECTION_ENGINE_RULES_URL,
body: { ...getCreateRulesSchemaMock(), type: 'unknown_type' },
body: { ...getPatchRulesSchemaMock(), type: 'unknown_type' },
});
const result = server.validate(request);
@ -188,5 +188,30 @@ describe('patch_rules', () => {
'Invalid value "unknown_type" supplied to "type"'
);
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'patch',
path: DETECTION_ENGINE_RULES_URL,
body: { from: 'now-7m', interval: '5m', ...getPatchRulesSchemaMock() },
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'patch',
path: DETECTION_ENGINE_RULES_URL,
body: {
from: 'now-3755555555555555.67s',
interval: '5m',
...getPatchRulesSchemaMock(),
},
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -154,5 +154,33 @@ describe('update_rules_bulk', () => {
'Invalid value "unknown_type" supplied to "type"'
);
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'put',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
body: [{ from: 'now-7m', interval: '5m', ...getCreateRulesSchemaMock(), type: 'query' }],
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'put',
path: `${DETECTION_ENGINE_RULES_URL}/_bulk_update`,
body: [
{
from: 'now-3755555555555555.67s',
interval: '5m',
...getCreateRulesSchemaMock(),
type: 'query',
},
],
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -19,7 +19,7 @@ import { requestContextMock, serverMock, requestMock } from '../__mocks__';
import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants';
import { updateRulesNotifications } from '../../rules/update_rules_notifications';
import { updateRulesRoute } from './update_rules_route';
import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/create_rules_schema.mock';
import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/update_rules_schema.mock';
jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create());
jest.mock('../../rules/update_rules_notifications');
@ -130,7 +130,7 @@ describe('update_rules', () => {
method: 'put',
path: DETECTION_ENGINE_RULES_URL,
body: {
...getCreateRulesSchemaMock(),
...getUpdateRulesSchemaMock(),
rule_id: undefined,
},
});
@ -145,7 +145,7 @@ describe('update_rules', () => {
const request = requestMock.create({
method: 'put',
path: DETECTION_ENGINE_RULES_URL,
body: { ...getCreateRulesSchemaMock(), type: 'query' },
body: { ...getUpdateRulesSchemaMock(), type: 'query' },
});
const result = await server.validate(request);
@ -156,7 +156,7 @@ describe('update_rules', () => {
const request = requestMock.create({
method: 'put',
path: DETECTION_ENGINE_RULES_URL,
body: { ...getCreateRulesSchemaMock(), type: 'unknown type' },
body: { ...getUpdateRulesSchemaMock(), type: 'unknown type' },
});
const result = await server.validate(request);
@ -164,5 +164,31 @@ describe('update_rules', () => {
'Invalid value "unknown type" supplied to "type"'
);
});
test('allows rule type of query and custom from and interval', async () => {
const request = requestMock.create({
method: 'put',
path: DETECTION_ENGINE_RULES_URL,
body: { from: 'now-7m', interval: '5m', ...getUpdateRulesSchemaMock(), type: 'query' },
});
const result = server.validate(request);
expect(result.ok).toHaveBeenCalled();
});
test('disallows invalid "from" param on rule', async () => {
const request = requestMock.create({
method: 'put',
path: DETECTION_ENGINE_RULES_URL,
body: {
from: 'now-3755555555555555.67s',
interval: '5m',
...getUpdateRulesSchemaMock(),
type: 'query',
},
});
const result = server.validate(request);
expect(result.badRequest).toHaveBeenCalledWith('Failed to parse "from" on rule param');
});
});
});

View file

@ -16,8 +16,8 @@ import {
getListsClient,
getExceptions,
sortExceptionItems,
parseScheduleDates,
} from './utils';
import { parseScheduleDates } from '../../../../common/detection_engine/utils';
import { RuleExecutorOptions } from './types';
import { searchAfterAndBulkCreate } from './search_after_bulk_create';
import { scheduleNotificationActions } from '../notifications/schedule_notification_actions';
@ -37,6 +37,7 @@ jest.mock('./utils');
jest.mock('../notifications/schedule_notification_actions');
jest.mock('./find_ml_signals');
jest.mock('./bulk_create_ml_signals');
jest.mock('./../../../../common/detection_engine/utils');
const getPayload = (ruleAlert: RuleAlertType, services: AlertServicesMock) => ({
alertId: ruleAlert.id,

View file

@ -14,6 +14,7 @@ import {
SERVER_APP_ID,
} from '../../../../common/constants';
import { isJobStarted, isMlRule } from '../../../../common/machine_learning/helpers';
import { parseScheduleDates } from '../../../../common/detection_engine/utils';
import { SetupPlugins } from '../../../plugin';
import { getInputIndex } from './get_input_output_index';
import {
@ -24,7 +25,6 @@ import { getFilter } from './get_filter';
import { SignalRuleAlertTypeDefinition, RuleAlertAttributes } from './types';
import {
getGapBetweenRuns,
parseScheduleDates,
getListsClient,
getExceptions,
getGapMaxCatchupRatio,

View file

@ -13,11 +13,11 @@ import { buildRuleMessageFactory } from './rule_messages';
import { ExceptionListClient } from '../../../../../lists/server';
import { getListArrayMock } from '../../../../common/detection_engine/schemas/types/lists.mock';
import { getExceptionListItemSchemaMock } from '../../../../../lists/common/schemas/response/exception_list_item_schema.mock';
import { parseScheduleDates } from '../../../../common/detection_engine/utils';
import {
generateId,
parseInterval,
parseScheduleDates,
getDriftTolerance,
getGapBetweenRuns,
getGapMaxCatchupRatio,

View file

@ -14,7 +14,7 @@ import { ExceptionListItemSchema } from '../../../../../lists/common/schemas';
import { ListArrayOrUndefined } from '../../../../common/detection_engine/schemas/types/lists';
import { BulkResponse, BulkResponseErrorAggregation, isValidUnit } from './types';
import { BuildRuleMessage } from './rule_messages';
import { hasLargeValueList } from '../../../../common/detection_engine/utils';
import { hasLargeValueList, parseScheduleDates } from '../../../../common/detection_engine/utils';
import { MAX_EXCEPTION_LIST_SIZE } from '../../../../../lists/common/constants';
interface SortExceptionsReturn {
@ -220,18 +220,6 @@ export const parseInterval = (intervalString: string): moment.Duration | null =>
}
};
export const parseScheduleDates = (time: string): moment.Moment | null => {
const isValidDateString = !isNaN(Date.parse(time));
const isValidInput = isValidDateString || time.trim().startsWith('now');
const formattedDate = isValidDateString
? moment(time)
: isValidInput
? dateMath.parse(time)
: null;
return formattedDate ?? null;
};
export const getDriftTolerance = ({
from,
to,

View file

@ -141,6 +141,43 @@ export default ({ getService }: FtrProviderContext): void => {
expect(bodyToCompare).to.eql(getSimpleRuleOutput('rule-1'));
});
it('should fail validation when importing a rule with malformed "from" params on the rules', async () => {
const stringifiedRule = JSON.stringify({
from: 'now-3755555555555555.67s',
interval: '5m',
...getSimpleRule('rule-1'),
});
const fileNdJson = Buffer.from(stringifiedRule + '\n');
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', fileNdJson, 'rules.ndjson')
.expect(200);
expect(body.errors[0].error.message).to.eql('Failed to parse "from" on rule param');
});
it('should fail validation when importing two rules and one has a malformed "from" params', async () => {
const stringifiedRule = JSON.stringify({
from: 'now-3755555555555555.67s',
interval: '5m',
...getSimpleRule('rule-1'),
});
const stringifiedRule2 = JSON.stringify({
...getSimpleRule('rule-2'),
});
const fileNdJson = Buffer.from([stringifiedRule, stringifiedRule2].join('\n'));
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', fileNdJson, 'rules.ndjson')
.expect(200);
// should result in one success and a failure message
expect(body.success_count).to.eql(1);
expect(body.errors[0].error.message).to.eql('Failed to parse "from" on rule param');
});
it('should be able to import two rules', async () => {
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)