[Security Solutions][Detection Engine] Adds ability to ignore fields …

…during alert indexing and a workaround for an EQL bug (#110927) (#111156) ## Summary Adds a workaround for EQL bug: elastic/elasticsearch#77152 Adds the safety feature mentioned here: #110802 Adds the ability to ignore particular [fields](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#search-fields-param) when the field is merged with [_source](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#source-filtering). Also fixes an EQL bug where EQL is introducing the meta field of `_ignored` within the fields and causing documents to not be indexable when we merge with the fields from EQL. Alerting document creation uses the fields API to get [runtime field](https://www.elastic.co/guide/en/elasticsearch/reference/current/runtime.html), [constant keyword](https://www.elastic.co/guide/en/elasticsearch/reference/master/keyword.html#constant-keyword-field-type), etc... that are only available within the [fields API](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-fields.html#search-fields-param) and then merges the field values not found within the `_source` document with the `_source` document and then finally indexes this merged document as an alert document. This fix/ability is a "safety feature" in that if a problematic [runtime field](https://www.elastic.co/guide/en/elasticsearch/reference/current/runtime.html), [constant keyword](https://www.elastic.co/guide/en/elasticsearch/reference/master/keyword.html#constant-keyword-field-type) is discovered or another bug along the stack we can set a `kibana.yml` key/value pair to ignore the problematic field. This _WILL NOT_ remove problematic fields from the `_source` document. This will only ignore problematic constant keyword, runtime fields, aliases, or anything else found in the fields API that is causing merge issues. This PR: * Adds a `alertIgnoreFields` `kibana.yml` array key with a default of an empty array if not specified. * Plumbs the `alertIgnoreFields` through the stack and into the fields/_source merge strategies of `missingFields` and `allFields` * Adds a temporary `isEqlBug77152` where it hard codes an ignore of `_ignored` until the EQL problem is fixed and then we will remove the workaround * Adds unit tests * Adds e2e tests which covers the described use cases above. The `alertIgnoreFields` key/value within `kibana.yml` if set should be an array of strings of each field you want to ignore. This can also contain regular expressions as long as they are of the form, `"/regex/"` in the array. Example if you want to ignore fields that are problematic called "host.name" and then one in which you want to ignore all fields that start with "user." using a regular expression: ```yml xpack.securitySolution.alertIgnoreFields: ['host.name', '/user\..*/'] ``` Although there are e2e tests which exercise the use cases... If you want to manual test the EQL bug fix you would add these documents in dev tools: ```json # Delete and add a mapping with a small ignore_above. DELETE eql-issue-ignore-fields-delme PUT eql-issue-ignore-fields-delme { "mappings" : { "dynamic": "strict", "properties" : { "@timestamp": { "type": "date" }, "some_keyword" : { "ignore_above": 5, "type" : "keyword" }, "other_keyword" : { "ignore_above": 10, "type" : "keyword" } } } } # Add a single document with one field that will be truncated and a second that will not. PUT eql-issue-ignore-fields-delme/_doc/1 { "@timestamp": "2021-09-02T04:13:05.626Z", "some_keyword": "longer than normal", "other_keyword": "normal" } ``` Then create an alert which queries everything from it: <img width="1155" alt="Screen Shot 2021-09-01 at 10 15 06 PM" src="https://user-images.githubusercontent.com/1151048/131781042-faa424cf-65a5-4ebb-b801-3f188940c81d.png"> and ensure signals are created: <img width="2214" alt="Screen Shot 2021-09-01 at 10 30 18 PM" src="https://user-images.githubusercontent.com/1151048/131782069-b9ab959c-f22d-44d5-baf0-561fe349c037.png"> To test the manual exclusions of any other problematic fields, create any index which has runtime fields or `constant keywords` but does not have anything within the `_source` document using dev tools. For example you can use `constant keyword` like so ```json PUT constant-keywords-deleme { "mappings": { "dynamic": "strict", "properties": { "@timestamp": { "type": "date" }, "testing_ignored": { "properties": { "constant": { "type": "constant_keyword", "value": "constant_value" } } }, "testing_regex": { "type": "constant_keyword", "value": "constant_value" }, "normal_constant": { "type": "constant_keyword", "value": "constant_value" }, "small_field": { "type": "keyword", "ignore_above": 10 } } } } PUT constant-keywords-deleme/_doc/1 { "@timestamp": "2021-09-02T04:20:01.760Z" } ``` Set in your `kibana.yml` the key/value of: ```yml xpack.securitySolution.alertIgnoreFields: ['testing_ignored.constant', '/.*_regex/'] ``` Setup a rule to run: <img width="1083" alt="Screen Shot 2021-09-01 at 10 23 23 PM" src="https://user-images.githubusercontent.com/1151048/131781696-fea0d421-836f-465c-9be6-5289fbb622a4.png"> Once it runs you should notice that the constant values for testing are not on the signals table since it only typically exists in the fields API: <img width="1166" alt="Screen Shot 2021-09-01 at 10 26 16 PM" src="https://user-images.githubusercontent.com/1151048/131781782-1684fb1d-bed9-4cf0-be9a-0abe1f0f34d1.png"> But the normal one still exists: <img width="1136" alt="Screen Shot 2021-09-01 at 10 26 31 PM" src="https://user-images.githubusercontent.com/1151048/131781827-5450c693-de9e-4285-b082-9f7a2cbd5d07.png"> If you change the `xpack.securitySolution.alertIgnoreFields` by removing it and re-generate the signals you will see these values added back. ### Checklist Delete any items that are not applicable to this PR. - [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 - [x] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/master/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) Co-authored-by: Frank Hassanabad <[email protected]>
elastic · Sep 3, 2021 · 9cf311a · 9cf311a
1 parent 8638e48
commit 9cf311a
Show file tree

Hide file tree

Showing 40 changed files with 1,044 additions and 232 deletions.
diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker
@@ -389,6 +389,7 @@ kibana_vars=(
     xpack.security.session.lifespan
     xpack.security.sessionTimeout
     xpack.securitySolution.alertMergeStrategy
+    xpack.securitySolution.alertIgnoreFields
     xpack.securitySolution.endpointResultListDefaultFirstPageIndex
     xpack.securitySolution.endpointResultListDefaultPageSize
     xpack.securitySolution.maxRuleImportExportSize

diff --git a/x-pack/plugins/security_solution/server/config.test.ts b/x-pack/plugins/security_solution/server/config.test.ts
@@ -0,0 +1,80 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { configSchema } from './config';
+
+describe('config', () => {
+  describe('alertIgnoreFields', () => {
+    test('should default to an empty array', () => {
+      expect(configSchema.validate({}).alertIgnoreFields).toEqual([]);
+    });
+
+    test('should accept an array of strings', () => {
+      expect(
+        configSchema.validate({ alertIgnoreFields: ['foo.bar', 'mars.bar'] }).alertIgnoreFields
+      ).toEqual(['foo.bar', 'mars.bar']);
+    });
+
+    test('should throw if a non string is being sent in', () => {
+      expect(
+        () =>
+          configSchema.validate({
+            alertIgnoreFields: 5,
+          }).alertIgnoreFields
+      ).toThrow('[alertIgnoreFields]: expected value of type [array] but got [number]');
+    });
+
+    test('should throw if we send in an invalid regular expression as a string', () => {
+      expect(
+        () =>
+          configSchema.validate({
+            alertIgnoreFields: ['/(/'],
+          }).alertIgnoreFields
+      ).toThrow(
+        '[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0'
+      );
+    });
+
+    test('should throw with two errors if we send two invalid regular expressions', () => {
+      expect(
+        () =>
+          configSchema.validate({
+            alertIgnoreFields: ['/(/', '/(invalid/'],
+          }).alertIgnoreFields
+      ).toThrow(
+        '[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0. "Invalid regular expression: /(invalid/: Unterminated group" at array position 1'
+      );
+    });
+
+    test('should throw with two errors with a valid string mixed in if we send two invalid regular expressions', () => {
+      expect(
+        () =>
+          configSchema.validate({
+            alertIgnoreFields: ['/(/', 'valid.string', '/(invalid/'],
+          }).alertIgnoreFields
+      ).toThrow(
+        '[alertIgnoreFields]: "Invalid regular expression: /(/: Unterminated group" at array position 0. "Invalid regular expression: /(invalid/: Unterminated group" at array position 2'
+      );
+    });
+
+    test('should accept a valid regular expression within the string', () => {
+      expect(
+        configSchema.validate({
+          alertIgnoreFields: ['/(.*)/'],
+        }).alertIgnoreFields
+      ).toEqual(['/(.*)/']);
+    });
+
+    test('should accept two valid regular expressions', () => {
+      expect(
+        configSchema.validate({
+          alertIgnoreFields: ['/(.*)/', '/(.valid*)/'],
+        }).alertIgnoreFields
+      ).toEqual(['/(.*)/', '/(.valid*)/']);
+    });
+  });
+});
diff --git a/x-pack/plugins/security_solution/server/config.ts b/x-pack/plugins/security_solution/server/config.ts
@@ -21,12 +21,61 @@ export const configSchema = schema.object({
   maxRuleImportPayloadBytes: schema.number({ defaultValue: 10485760 }),
   maxTimelineImportExportSize: schema.number({ defaultValue: 10000 }),
   maxTimelineImportPayloadBytes: schema.number({ defaultValue: 10485760 }),
+
+  /**
+   * This is used within the merge strategies:
+   * server/lib/detection_engine/signals/source_fields_merging
+   *
+   * For determining which strategy for merging "fields" and "_source" together to get
+   * runtime fields, constant keywords, etc...
+   *
+   * "missingFields" (default) This will only merge fields that are missing from the _source and exist in the fields.
+   * "noFields" This will turn off all merging of runtime fields, constant keywords from fields.
+   * "allFields" This will merge and overwrite anything found within "fields" into "_source" before indexing the data.
+   */
   alertMergeStrategy: schema.oneOf(
     [schema.literal('allFields'), schema.literal('missingFields'), schema.literal('noFields')],
     {
       defaultValue: 'missingFields',
     }
   ),
+
+  /**
+   * This is used within the merge strategies:
+   * server/lib/detection_engine/signals/source_fields_merging
+   *
+   * For determining if we need to ignore particular "fields" and not merge them with "_source" such as
+   * runtime fields, constant keywords, etc...
+   *
+   * This feature and functionality is mostly as "safety feature" meaning that we have had bugs in the past
+   * where something down the stack unexpectedly ends up in the fields API which causes documents to not
+   * be indexable. Rather than changing alertMergeStrategy to be "noFields", you can use this array to add
+   * any problematic values.
+   *
+   * You can use plain dotted notation strings such as "host.name" or a regular expression such as "/host\..+/"
+   */
+  alertIgnoreFields: schema.arrayOf(schema.string(), {
+    defaultValue: [],
+    validate(ignoreFields) {
+      const errors = ignoreFields.flatMap((ignoreField, index) => {
+        if (ignoreField.startsWith('/') && ignoreField.endsWith('/')) {
+          try {
+            new RegExp(ignoreField.slice(1, -1));
+            return [];
+          } catch (error) {
+            return [`"${error.message}" at array position ${index}`];
+          }
+        } else {
+          return [];
+        }
+      });
+      if (errors.length !== 0) {
+        return errors.join('. ');
+      } else {
+        return undefined;
+      }
+    },
+  }),
   [SIGNALS_INDEX_KEY]: schema.string({ defaultValue: DEFAULT_SIGNALS_INDEX }),
 
   /**

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/index.ts
@@ -26,6 +26,7 @@ export const createMockConfig = (): ConfigType => ({
   endpointResultListDefaultPageSize: 10,
   packagerTaskInterval: '60s',
   alertMergeStrategy: 'missingFields',
+  alertIgnoreFields: [],
   prebuiltRulesFromFileSystem: true,
   prebuiltRulesFromSavedObjects: false,
 });

diff --git a/...rity_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts b/...rity_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts
@@ -40,6 +40,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({
   lists,
   logger,
   mergeStrategy,
+  ignoreFields,
   ruleDataClient,
   ruleDataService,
 }) => (type) => {
@@ -208,6 +209,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({
 
         const wrapHits = wrapHitsFactory({
           logger,
+          ignoreFields,
           mergeStrategy,
           ruleSO,
           spaceId,

diff --git a/...curity_solution/server/lib/detection_engine/rule_types/factories/utils/build_bulk_body.ts b/...curity_solution/server/lib/detection_engine/rule_types/factories/utils/build_bulk_body.ts
@@ -36,10 +36,11 @@ export const buildBulkBody = (
   ruleSO: SavedObject<AlertAttributes>,
   doc: SignalSourceHit,
   mergeStrategy: ConfigType['alertMergeStrategy'],
+  ignoreFields: ConfigType['alertIgnoreFields'],
   applyOverrides: boolean,
   buildReasonMessage: BuildReasonMessage
 ): RACAlert => {
-  const mergedDoc = getMergeStrategy(mergeStrategy)({ doc });
+  const mergedDoc = getMergeStrategy(mergeStrategy)({ doc, ignoreFields });
   const rule = applyOverrides
     ? buildRuleWithOverrides(ruleSO, mergedDoc._source ?? {})
     : buildRuleWithoutOverrides(ruleSO);

diff --git a/...s/security_solution/server/lib/detection_engine/rule_types/factories/wrap_hits_factory.ts b/...s/security_solution/server/lib/detection_engine/rule_types/factories/wrap_hits_factory.ts
@@ -7,7 +7,7 @@
 
 import { Logger } from 'kibana/server';
 
-import { SearchAfterAndBulkCreateParams, SignalSourceHit, WrapHits } from '../../signals/types';
+import { SearchAfterAndBulkCreateParams, WrapHits } from '../../signals/types';
 import { buildBulkBody } from './utils/build_bulk_body';
 import { generateId } from '../../signals/utils';
 import { filterDuplicateSignals } from '../../signals/filter_duplicate_signals';
@@ -16,13 +16,15 @@ import { WrappedRACAlert } from '../types';
 
 export const wrapHitsFactory = ({
   logger,
+  ignoreFields,
   mergeStrategy,
   ruleSO,
   spaceId,
 }: {
   logger: Logger;
   ruleSO: SearchAfterAndBulkCreateParams['ruleSO'];
   mergeStrategy: ConfigType['alertMergeStrategy'];
+  ignoreFields: ConfigType['alertIgnoreFields'];
   spaceId: string | null | undefined;
 }): WrapHits => (events, buildReasonMessage) => {
   try {
@@ -38,8 +40,9 @@ export const wrapHitsFactory = ({
         _source: buildBulkBody(
           spaceId,
           ruleSO,
-          doc as SignalSourceHit,
+          doc,
           mergeStrategy,
+          ignoreFields,
           true,
           buildReasonMessage
         ),

diff --git a/...lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.test.ts b/...lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.test.ts
@@ -56,6 +56,7 @@ describe('Indicator Match Alerts', () => {
       experimentalFeatures: allowedExperimentalValues,
       lists: dependencies.lists,
       logger: dependencies.logger,
+      ignoreFields: [],
       mergeStrategy: 'allFields',
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,
@@ -97,6 +98,7 @@ describe('Indicator Match Alerts', () => {
       lists: dependencies.lists,
       logger: dependencies.logger,
       mergeStrategy: 'allFields',
+      ignoreFields: [],
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,
       version: '1.0.0',
@@ -135,6 +137,7 @@ describe('Indicator Match Alerts', () => {
       lists: dependencies.lists,
       logger: dependencies.logger,
       mergeStrategy: 'allFields',
+      ignoreFields: [],
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,
       version: '1.0.0',

diff --git a/...rver/lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.ts b/...rver/lib/detection_engine/rule_types/indicator_match/create_indicator_match_alert_type.ts
@@ -19,6 +19,7 @@ export const createIndicatorMatchAlertType = (createOptions: CreateRuleOptions)
     lists,
     logger,
     mergeStrategy,
+    ignoreFields,
     ruleDataClient,
     version,
     ruleDataService,
@@ -27,6 +28,7 @@ export const createIndicatorMatchAlertType = (createOptions: CreateRuleOptions)
     lists,
     logger,
     mergeStrategy,
+    ignoreFields,
     ruleDataClient,
     ruleDataService,
   });

diff --git a/.../security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.test.ts b/.../security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.test.ts
@@ -98,6 +98,7 @@ describe('Machine Learning Alerts', () => {
       lists: dependencies.lists,
       logger: dependencies.logger,
       mergeStrategy: 'allFields',
+      ignoreFields: [],
       ml: mlMock,
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,

diff --git a/...ugins/security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.ts b/...ugins/security_solution/server/lib/detection_engine/rule_types/ml/create_ml_alert_type.ts
@@ -14,11 +14,20 @@ import { createSecurityRuleTypeFactory } from '../create_security_rule_type_fact
 import { CreateRuleOptions } from '../types';
 
 export const createMlAlertType = (createOptions: CreateRuleOptions) => {
-  const { lists, logger, mergeStrategy, ml, ruleDataClient, ruleDataService } = createOptions;
+  const {
+    lists,
+    logger,
+    mergeStrategy,
+    ignoreFields,
+    ml,
+    ruleDataClient,
+    ruleDataService,
+  } = createOptions;
   const createSecurityRuleType = createSecurityRuleTypeFactory({
     lists,
     logger,
     mergeStrategy,
+    ignoreFields,
     ruleDataClient,
     ruleDataService,
   });

diff --git a/...ity_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.test.ts b/...ity_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.test.ts
@@ -32,6 +32,7 @@ describe('Custom query alerts', () => {
       lists: dependencies.lists,
       logger: dependencies.logger,
       mergeStrategy: 'allFields',
+      ignoreFields: [],
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,
       version: '1.0.0',
@@ -79,6 +80,7 @@ describe('Custom query alerts', () => {
       lists: dependencies.lists,
       logger: dependencies.logger,
       mergeStrategy: 'allFields',
+      ignoreFields: [],
       ruleDataClient: dependencies.ruleDataClient,
       ruleDataService: dependencies.ruleDataService,
       version: '1.0.0',

diff --git a/...security_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.ts b/...security_solution/server/lib/detection_engine/rule_types/query/create_query_alert_type.ts
@@ -19,6 +19,7 @@ export const createQueryAlertType = (createOptions: CreateRuleOptions) => {
     lists,
     logger,
     mergeStrategy,
+    ignoreFields,
     ruleDataClient,
     version,
     ruleDataService,
@@ -27,6 +28,7 @@ export const createQueryAlertType = (createOptions: CreateRuleOptions) => {
     lists,
     logger,
     mergeStrategy,
+    ignoreFields,
     ruleDataClient,
     ruleDataService,
   });

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/types.ts
@@ -96,6 +96,7 @@ export type CreateSecurityRuleTypeFactory = (options: {
   lists: SetupPlugins['lists'];
   logger: Logger;
   mergeStrategy: ConfigType['alertMergeStrategy'];
+  ignoreFields: ConfigType['alertIgnoreFields'];
   ruleDataClient: IRuleDataClient;
   ruleDataService: IRuleDataPluginService;
 }) => <
@@ -124,6 +125,7 @@ export interface CreateRuleOptions {
   lists: SetupPlugins['lists'];
   logger: Logger;
   mergeStrategy: ConfigType['alertMergeStrategy'];
+  ignoreFields: ConfigType['alertIgnoreFields'];
   ml?: SetupPlugins['ml'];
   ruleDataClient: IRuleDataClient;
   version: string;