[Security Solution] Fix rules table filtering by tags with special ch…

…aracters (#163767) **Fixes: #163692 ## Summary This PR fixes rules table filtering for tags with special characters like colon `category:tag`. We didn't have this bug in `8.9` it was introduced by #160480 in `8.10` scope. ## Details Coverage Overview Dashboard required very similar rule filtering functionality as rule table has so #160480 moved the common logic to `common` folder and started using `escapeKuery` from `@kbn/es-query` instead of custom `escapeKuery` defined in Security Solution in `x-pack/plugins/security_solution/public/common/lib/kuery/index.ts`. The difference is the first function escapes all the special characters `\():<>"*` but the second escapes only quotes but the value must be enclosed in quotes to be passed on. Both ways are correct according to [docs](https://www.elastic.co/guide/en/kibana/current/kuery-query.html). So escaping all the special characters and enclosing the value in quotes breaks rules search. ### How was it fixed? Escaping related code in `x-pack/plugins/security_solution/common/utils/kql.ts` (`convertRulesFilterToKQL` in particular) was moved to `x-pack/plugins/security_solution/common/detection_engine/rule_management/rule_filtering.ts` as Coverage Overview Dashboard API endpoint and rules table UI share the same KQL helpers and refactored along the way to be much more transparent. ### 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
elastic · Aug 28, 2023 · 92ff716 · 92ff716
1 parent 53173f1
commit 92ff716
Show file tree

Hide file tree

Showing 19 changed files with 376 additions and 314 deletions.
diff --git a/...s/security_solution/common/rule_fields.ts → ...ion_engine/rule_management/rule_fields.ts b/...s/security_solution/common/rule_fields.ts → ...ion_engine/rule_management/rule_fields.ts
diff --git a/.../plugins/security_solution/common/detection_engine/rule_management/rule_filtering.test.ts b/.../plugins/security_solution/common/detection_engine/rule_management/rule_filtering.test.ts
@@ -0,0 +1,89 @@
+/*
+ * 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 { convertRulesFilterToKQL } from './rule_filtering';
+
+describe('convertRulesFilterToKQL', () => {
+  const filterOptions = {
+    filter: '',
+    showCustomRules: false,
+    showElasticRules: false,
+    tags: [],
+  };
+
+  it('returns empty string if filter options are empty', () => {
+    const kql = convertRulesFilterToKQL(filterOptions);
+
+    expect(kql).toBe('');
+  });
+
+  it('handles presence of "filter" properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, filter: 'foo' });
+
+    expect(kql).toBe(
+      '(alert.attributes.name: "foo" OR alert.attributes.params.index: "foo" OR alert.attributes.params.threat.tactic.id: "foo" OR alert.attributes.params.threat.tactic.name: "foo" OR alert.attributes.params.threat.technique.id: "foo" OR alert.attributes.params.threat.technique.name: "foo" OR alert.attributes.params.threat.technique.subtechnique.id: "foo" OR alert.attributes.params.threat.technique.subtechnique.name: "foo")'
+    );
+  });
+
+  it('escapes "filter" value properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, filter: '" OR (foo: bar)' });
+
+    expect(kql).toBe(
+      '(alert.attributes.name: "\\" OR (foo: bar)" OR alert.attributes.params.index: "\\" OR (foo: bar)" OR alert.attributes.params.threat.tactic.id: "\\" OR (foo: bar)" OR alert.attributes.params.threat.tactic.name: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.id: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.name: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.subtechnique.id: "\\" OR (foo: bar)" OR alert.attributes.params.threat.technique.subtechnique.name: "\\" OR (foo: bar)")'
+    );
+  });
+
+  it('handles presence of "showCustomRules" properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, showCustomRules: true });
+
+    expect(kql).toBe(`alert.attributes.params.immutable: false`);
+  });
+
+  it('handles presence of "showElasticRules" properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, showElasticRules: true });
+
+    expect(kql).toBe(`alert.attributes.params.immutable: true`);
+  });
+
+  it('handles presence of "showElasticRules" and "showCustomRules" at the same time properly', () => {
+    const kql = convertRulesFilterToKQL({
+      ...filterOptions,
+      showElasticRules: true,
+      showCustomRules: true,
+    });
+
+    expect(kql).toBe('');
+  });
+
+  it('handles presence of "tags" properly', () => {
+    const kql = convertRulesFilterToKQL({ ...filterOptions, tags: ['tag1', 'tag2'] });
+
+    expect(kql).toBe('alert.attributes.tags:("tag1" AND "tag2")');
+  });
+
+  it('handles combination of different properties properly', () => {
+    const kql = convertRulesFilterToKQL({
+      ...filterOptions,
+      filter: 'foo',
+      showElasticRules: true,
+      tags: ['tag1', 'tag2'],
+    });
+
+    expect(kql).toBe(
+      `(alert.attributes.name: "foo" OR alert.attributes.params.index: "foo" OR alert.attributes.params.threat.tactic.id: "foo" OR alert.attributes.params.threat.tactic.name: "foo" OR alert.attributes.params.threat.technique.id: "foo" OR alert.attributes.params.threat.technique.name: "foo" OR alert.attributes.params.threat.technique.subtechnique.id: "foo" OR alert.attributes.params.threat.technique.subtechnique.name: "foo") AND alert.attributes.params.immutable: true AND alert.attributes.tags:(\"tag1\" AND \"tag2\")`
+    );
+  });
+
+  it('handles presence of "excludeRuleTypes" properly', () => {
+    const kql = convertRulesFilterToKQL({
+      ...filterOptions,
+      excludeRuleTypes: ['machine_learning', 'saved_query'],
+    });
+
+    expect(kql).toBe('NOT alert.attributes.params.type: ("machine_learning" OR "saved_query")');
+  });
+});
diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/rule_filtering.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/rule_filtering.ts
@@ -0,0 +1,113 @@
+/*
+ * 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 type { Type } from '@kbn/securitysolution-io-ts-alerting-types';
+import { RuleExecutionStatus } from '../../api/detection_engine';
+import { prepareKQLStringParam } from '../../utils/kql';
+import {
+  ENABLED_FIELD,
+  LAST_RUN_OUTCOME_FIELD,
+  PARAMS_IMMUTABLE_FIELD,
+  PARAMS_TYPE_FIELD,
+  RULE_NAME_FIELD,
+  RULE_PARAMS_FIELDS,
+  TAGS_FIELD,
+} from './rule_fields';
+
+export const KQL_FILTER_IMMUTABLE_RULES = `${PARAMS_IMMUTABLE_FIELD}: true`;
+export const KQL_FILTER_MUTABLE_RULES = `${PARAMS_IMMUTABLE_FIELD}: false`;
+export const KQL_FILTER_ENABLED_RULES = `${ENABLED_FIELD}: true`;
+export const KQL_FILTER_DISABLED_RULES = `${ENABLED_FIELD}: false`;
+
+interface RulesFilterOptions {
+  filter: string;
+  showCustomRules: boolean;
+  showElasticRules: boolean;
+  enabled: boolean;
+  tags: string[];
+  excludeRuleTypes: Type[];
+  ruleExecutionStatus: RuleExecutionStatus;
+}
+
+/**
+ * Convert rules filter options object to KQL query
+ *
+ * @param filterOptions desired filters (e.g. filter/sortField/sortOrder)
+ *
+ * @returns KQL string
+ */
+export function convertRulesFilterToKQL({
+  filter: searchTerm,
+  showCustomRules,
+  showElasticRules,
+  enabled,
+  tags,
+  excludeRuleTypes = [],
+  ruleExecutionStatus,
+}: Partial<RulesFilterOptions>): string {
+  const kql: string[] = [];
+
+  if (searchTerm?.length) {
+    kql.push(`(${convertRuleSearchTermToKQL(searchTerm)})`);
+  }
+
+  if (showCustomRules && showElasticRules) {
+    // if both showCustomRules && showElasticRules selected we omit filter, as it includes all existing rules
+  } else if (showElasticRules) {
+    kql.push(KQL_FILTER_IMMUTABLE_RULES);
+  } else if (showCustomRules) {
+    kql.push(KQL_FILTER_MUTABLE_RULES);
+  }
+
+  if (enabled !== undefined) {
+    kql.push(enabled ? KQL_FILTER_ENABLED_RULES : KQL_FILTER_DISABLED_RULES);
+  }
+
+  if (tags?.length) {
+    kql.push(convertRuleTagsToKQL(tags));
+  }
+
+  if (excludeRuleTypes.length) {
+    kql.push(`NOT ${convertRuleTypesToKQL(excludeRuleTypes)}`);
+  }
+
+  if (ruleExecutionStatus === RuleExecutionStatus.succeeded) {
+    kql.push(`${LAST_RUN_OUTCOME_FIELD}: "succeeded"`);
+  } else if (ruleExecutionStatus === RuleExecutionStatus['partial failure']) {
+    kql.push(`${LAST_RUN_OUTCOME_FIELD}: "warning"`);
+  } else if (ruleExecutionStatus === RuleExecutionStatus.failed) {
+    kql.push(`${LAST_RUN_OUTCOME_FIELD}: "failed"`);
+  }
+
+  return kql.join(' AND ');
+}
+
+const SEARCHABLE_RULE_ATTRIBUTES = [
+  RULE_NAME_FIELD,
+  RULE_PARAMS_FIELDS.INDEX,
+  RULE_PARAMS_FIELDS.TACTIC_ID,
+  RULE_PARAMS_FIELDS.TACTIC_NAME,
+  RULE_PARAMS_FIELDS.TECHNIQUE_ID,
+  RULE_PARAMS_FIELDS.TECHNIQUE_NAME,
+  RULE_PARAMS_FIELDS.SUBTECHNIQUE_ID,
+  RULE_PARAMS_FIELDS.SUBTECHNIQUE_NAME,
+];
+
+export function convertRuleSearchTermToKQL(
+  searchTerm: string,
+  attributes = SEARCHABLE_RULE_ATTRIBUTES
+): string {
+  return attributes.map((param) => `${param}: ${prepareKQLStringParam(searchTerm)}`).join(' OR ');
+}
+
+export function convertRuleTagsToKQL(tags: string[]): string {
+  return `${TAGS_FIELD}:(${tags.map(prepareKQLStringParam).join(' AND ')})`;
+}
+
+export function convertRuleTypesToKQL(ruleTypes: Type[]): string {
+  return `${PARAMS_TYPE_FIELD}: (${ruleTypes.map(prepareKQLStringParam).join(' OR ')})`;
+}
diff --git a/x-pack/plugins/security_solution/common/utils/kql.test.ts b/x-pack/plugins/security_solution/common/utils/kql.test.ts
@@ -5,85 +5,73 @@
  * 2.0.
  */
 
-import { convertRulesFilterToKQL } from './kql';
-
-describe('convertRulesFilterToKQL', () => {
-  const filterOptions = {
-    filter: '',
-    showCustomRules: false,
-    showElasticRules: false,
-    tags: [],
-  };
-
-  it('returns empty string if filter options are empty', () => {
-    const kql = convertRulesFilterToKQL(filterOptions);
-
-    expect(kql).toBe('');
-  });
-
-  it('handles presence of "filter" properly', () => {
-    const kql = convertRulesFilterToKQL({ ...filterOptions, filter: 'foo' });
-
-    expect(kql).toBe(
-      '(alert.attributes.name: "foo" OR alert.attributes.params.index: "foo" OR alert.attributes.params.threat.tactic.id: "foo" OR alert.attributes.params.threat.tactic.name: "foo" OR alert.attributes.params.threat.technique.id: "foo" OR alert.attributes.params.threat.technique.name: "foo" OR alert.attributes.params.threat.technique.subtechnique.id: "foo" OR alert.attributes.params.threat.technique.subtechnique.name: "foo")'
-    );
+import { escapeKQLStringParam, prepareKQLParam, prepareKQLStringParam } from './kql';
+
+const testCases = [
+  ['does NOT remove white spaces quotes', ' netcat', ' netcat'],
+  ['escapes quotes', 'I said, "Hello."', 'I said, \\"Hello.\\"'],
+  [
+    'should escape special characters',
+    `This \\ has (a lot of) <special> characters, don't you *think*? "Yes."`,
+    `This \\ has (a lot of) <special> characters, don't you *think*? \\"Yes.\\"`,
+  ],
+  ['does NOT escape keywords', 'foo and bar or baz not qux', 'foo and bar or baz not qux'],
+  [
+    'does NOT escape keywords next to each other',
+    'foo and bar or not baz',
+    'foo and bar or not baz',
+  ],
+  [
+    'does NOT escape keywords without surrounding spaces',
+    'And this has keywords, or does it not?',
+    'And this has keywords, or does it not?',
+  ],
+  [
+    'does NOT escape uppercase keywords',
+    'And this has keywords, or does it not?',
+    'And this has keywords, or does it not?',
+  ],
+  ['does NOT escape uppercase keywords', 'foo AND bar', 'foo AND bar'],
+  [
+    'escapes special characters and NOT keywords',
+    'Hello, "world", and <nice> to meet you!',
+    'Hello, \\"world\\", and <nice> to meet you!',
+  ],
+  [
+    'escapes newlines and tabs',
+    'This\nhas\tnewlines\r\nwith\ttabs',
+    'This\\nhas\\tnewlines\\r\\nwith\\ttabs',
+  ],
+];
+
+describe('prepareKQLParam', () => {
+  it.each(testCases)('%s', (_, input, expected) => {
+    expect(prepareKQLParam(input)).toBe(`"${expected}"`);
   });
 
-  it('escapes "filter" value properly', () => {
-    const kql = convertRulesFilterToKQL({ ...filterOptions, filter: '" OR (foo: bar)' });
+  it('stringifies numbers without enclosing by quotes', () => {
+    const input = 10;
+    const expected = '10';
 
-    expect(kql).toBe(
-      '(alert.attributes.name: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.index: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.tactic.id: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.tactic.name: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.technique.id: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.technique.name: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.technique.subtechnique.id: "\\" \\OR \\(foo\\: bar\\)" OR alert.attributes.params.threat.technique.subtechnique.name: "\\" \\OR \\(foo\\: bar\\)")'
-    );
+    expect(prepareKQLParam(input)).toBe(expected);
   });
 
-  it('handles presence of "showCustomRules" properly', () => {
-    const kql = convertRulesFilterToKQL({ ...filterOptions, showCustomRules: true });
+  it('stringifies booleans without enclosing by quotes', () => {
+    const input = true;
+    const expected = 'true';
 
-    expect(kql).toBe(`alert.attributes.params.immutable: false`);
+    expect(prepareKQLParam(input)).toBe(expected);
   });
+});
 
-  it('handles presence of "showElasticRules" properly', () => {
-    const kql = convertRulesFilterToKQL({ ...filterOptions, showElasticRules: true });
-
-    expect(kql).toBe(`alert.attributes.params.immutable: true`);
-  });
-
-  it('handles presence of "showElasticRules" and "showCustomRules" at the same time properly', () => {
-    const kql = convertRulesFilterToKQL({
-      ...filterOptions,
-      showElasticRules: true,
-      showCustomRules: true,
-    });
-
-    expect(kql).toBe('');
-  });
-
-  it('handles presence of "tags" properly', () => {
-    const kql = convertRulesFilterToKQL({ ...filterOptions, tags: ['tag1', 'tag2'] });
-
-    expect(kql).toBe('alert.attributes.tags:("tag1" AND "tag2")');
-  });
-
-  it('handles combination of different properties properly', () => {
-    const kql = convertRulesFilterToKQL({
-      ...filterOptions,
-      filter: 'foo',
-      showElasticRules: true,
-      tags: ['tag1', 'tag2'],
-    });
-
-    expect(kql).toBe(
-      `(alert.attributes.name: "foo" OR alert.attributes.params.index: "foo" OR alert.attributes.params.threat.tactic.id: "foo" OR alert.attributes.params.threat.tactic.name: "foo" OR alert.attributes.params.threat.technique.id: "foo" OR alert.attributes.params.threat.technique.name: "foo" OR alert.attributes.params.threat.technique.subtechnique.id: "foo" OR alert.attributes.params.threat.technique.subtechnique.name: "foo") AND alert.attributes.params.immutable: true AND alert.attributes.tags:(\"tag1\" AND \"tag2\")`
-    );
+describe('prepareKQLStringParam', () => {
+  it.each(testCases)('%s', (_, input, expected) => {
+    expect(prepareKQLStringParam(input)).toBe(`"${expected}"`);
   });
+});
 
-  it('handles presence of "excludeRuleTypes" properly', () => {
-    const kql = convertRulesFilterToKQL({
-      ...filterOptions,
-      excludeRuleTypes: ['machine_learning', 'saved_query'],
-    });
-
-    expect(kql).toBe('NOT alert.attributes.params.type: ("machine_learning" OR "saved_query")');
+describe('escapeKQLStringParam', () => {
+  it.each(testCases)('%s', (_, input, expected) => {
+    expect(escapeKQLStringParam(input)).toBe(expected);
   });
 });