[7.9] [Security Solution] [Detections] Updates rules routes to valida…

…te "from" param on rules (#76000) (#76048) * 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
elastic · Aug 27, 2020 · f3d36a1 · f3d36a1
1 parent 3512f57
commit f3d36a1
Show file tree

Hide file tree

Showing 15 changed files with 246 additions and 30 deletions.
diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts
@@ -7,11 +7,14 @@
 /* eslint-disable @typescript-eslint/camelcase */
 
 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]);

diff --git a/...ck/plugins/security_solution/common/detection_engine/schemas/types/default_from_string.ts b/...ck/plugins/security_solution/common/detection_engine/schemas/types/default_from_string.ts
@@ -6,15 +6,19 @@
 
 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
  */
 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
 );
diff --git a/x-pack/plugins/security_solution/common/detection_engine/utils.ts b/x-pack/plugins/security_solution/common/detection_engine/utils.ts
@@ -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;
+};
diff --git a/...urity_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts b/...urity_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts
@@ -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,

diff --git a/...ecurity_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.test.ts b/...ecurity_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.test.ts
@@ -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');
+    });
   });
 });
diff --git a/...ins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts b/...ins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts
@@ -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');
+    });
   });
 });
diff --git a/...security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.test.ts b/...security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.test.ts
@@ -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');
+    });
   });
 });
diff --git a/...gins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.test.ts b/...gins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.test.ts
@@ -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,13 +180,38 @@ 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);
 
       expect(result.badRequest).toHaveBeenCalledWith(
         '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');
+    });
   });
 });
diff --git a/...ecurity_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.test.ts b/...ecurity_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.test.ts
@@ -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');
+    });
   });
 });
diff --git a/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts b/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts
@@ -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,13 +156,39 @@ 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);
 
       expect(result.badRequest).toHaveBeenCalledWith(
         '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');
+    });
   });
 });
diff --git a/...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/...gins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts
@@ -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,