Skip to content

Commit

Permalink
[Security Solution][Detection Engine] adds alert suppression to ES|QL…
Browse files Browse the repository at this point in the history
… rule type (elastic#180927)

## Summary

- addresses elastic/security-team#9203
- adds alert suppression for new terms rule type
- similarly to [custom investigation
fields](elastic#177746) list of available
suppression fields:
  - shows only ES|QL fields returned in query for aggregating queries
- shows ES|QL fields returned in query + index fields for
non-aggregating queries. Since resulted alerts for this type of query,
are enriched with source documents.

### Demo

1. run esql rule w/o suppression
2. run esql rule w/ suppression per rule execution. Since ES|QL query is
aggregating, no alerts suppressed on already agrregated field `host.ip`
3. run suppression on interval 20m
4. run suppression for custom ES|QL field which is the same as
`host.ip`, hence same results
5. run suppression on interval 100m


https://github.com/elastic/kibana/assets/92328789/4bd8cf13-6e23-4842-b775-605c74ae0127

### Limitations

Since suppressed alerts deduplication relies on alert timestamps,
sorting of results other than `@timestamp asc` in ES|QL query may impact
on number of suppressed alerts, when number of possible alerts more than
max_signals.
This affects only non-aggregating queries, since suppression boundaries
for these alerts set as rule execution time

### Checklist

- [x] Functional changes are hidden behind a feature flag 

    Feature flag `alertSuppressionForEsqlRuleEnabled`

- [x] Functional changes are covered with a test plan and automated
tests.

  - elastic/security-team#9389

- [x] Stability of new and changed tests is verified using the [Flaky
Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner).
- FTR(x100):
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5907
- Cypress(x100):
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6011
  
- [x] Comprehensive manual testing is done by two engineers: the PR
author and one of the PR reviewers. Changes are tested in both ESS and
Serverless.

- [x] Mapping changes are accompanied by a technical design document. It
can be a GitHub issue or an RFC explaining the changes. The design
document is shared with and approved by the appropriate teams and
individual stakeholders.

Existing AlertSuppression schema field is used for ES|QL rule, the one
that already used for Query, New terms and IM rules.
  
  ```yml
      alert_suppression:
$ref:
'./common_attributes.schema.yaml#/components/schemas/AlertSuppression'
  ```
  where
  
  ```yml
      AlertSuppression:
        type: object
        properties:
          group_by:
            $ref: '#/components/schemas/AlertSuppressionGroupBy'
          duration:
            $ref: '#/components/schemas/AlertSuppressionDuration'
          missing_fields_strategy:
$ref: '#/components/schemas/AlertSuppressionMissingFieldsStrategy'
        required:
          - group_by
     ```

- [x] Functional changes are communicated to the Docs team. A ticket or
PR is opened in https://github.com/elastic/security-docs. The following
information is included: any feature flags used, affected environments
(Serverless, ESS, or both).

  - elastic/security-docs#5156

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Nikita Indik <[email protected]>
  • Loading branch information
3 people authored May 20, 2024
1 parent 8b7fa0d commit 6e6b99c
Show file tree
Hide file tree
Showing 55 changed files with 3,758 additions and 565 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,9 @@ describe('getIndexListFromEsqlQuery', () => {
getIndexPatternFromESQLQueryMock.mockReturnValue('test-1 , test-2 ');
expect(getIndexListFromEsqlQuery('From test-1, test-2 ')).toEqual(['test-1', 'test-2']);
});

it('should return empty array when getIndexPatternFromESQLQuery throws error', () => {
getIndexPatternFromESQLQueryMock.mockReturnValue(new Error('Fail to parse'));
expect(getIndexListFromEsqlQuery('From test-1 []')).toEqual([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ import { getIndexPatternFromESQLQuery } from '@kbn/esql-utils';
* parses ES|QL query and returns array of indices
*/
export const getIndexListFromEsqlQuery = (query: string | undefined): string[] => {
const indexString = getIndexPatternFromESQLQuery(query);
try {
const indexString = getIndexPatternFromESQLQuery(query);

return getIndexListFromIndexString(indexString);
return getIndexListFromIndexString(indexString);
} catch (e) {
return [];
}
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,7 @@ describe('rules schema', () => {
// behaviour common for multiple rule types
const cases = [
{ ruleType: 'threat_match', ruleMock: getCreateThreatMatchRulesSchemaMock() },
{ ruleType: 'esql', ruleMock: getCreateEsqlRulesSchemaMock() },
{ ruleType: 'query', ruleMock: getCreateRulesSchemaMock() },
{ ruleType: 'saved_query', ruleMock: getCreateSavedQueryRulesSchemaMock() },
{ ruleType: 'eql', ruleMock: getCreateEqlRuleSchemaMock() },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,14 +560,19 @@ export const EsqlRuleRequiredFields = z.object({
query: RuleQuery,
});

export type EsqlRuleOptionalFields = z.infer<typeof EsqlRuleOptionalFields>;
export const EsqlRuleOptionalFields = z.object({
alert_suppression: AlertSuppression.optional(),
});

export type EsqlRulePatchFields = z.infer<typeof EsqlRulePatchFields>;
export const EsqlRulePatchFields = EsqlRuleRequiredFields.partial();
export const EsqlRulePatchFields = EsqlRuleOptionalFields.merge(EsqlRuleRequiredFields.partial());

export type EsqlRuleResponseFields = z.infer<typeof EsqlRuleResponseFields>;
export const EsqlRuleResponseFields = EsqlRuleRequiredFields;
export const EsqlRuleResponseFields = EsqlRuleOptionalFields.merge(EsqlRuleRequiredFields);

export type EsqlRuleCreateFields = z.infer<typeof EsqlRuleCreateFields>;
export const EsqlRuleCreateFields = EsqlRuleRequiredFields;
export const EsqlRuleCreateFields = EsqlRuleOptionalFields.merge(EsqlRuleRequiredFields);

export type EsqlRule = z.infer<typeof EsqlRule>;
export const EsqlRule = SharedResponseProps.merge(EsqlRuleResponseFields);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -826,17 +826,26 @@ components:
- language
- query

EsqlRuleOptionalFields:
type: object
properties:
alert_suppression:
$ref: './common_attributes.schema.yaml#/components/schemas/AlertSuppression'

EsqlRulePatchFields:
allOf:
- $ref: '#/components/schemas/EsqlRuleOptionalFields'
- $ref: '#/components/schemas/EsqlRuleRequiredFields'
x-modify: partial

EsqlRuleResponseFields:
allOf:
- $ref: '#/components/schemas/EsqlRuleOptionalFields'
- $ref: '#/components/schemas/EsqlRuleRequiredFields'

EsqlRuleCreateFields:
allOf:
- $ref: '#/components/schemas/EsqlRuleOptionalFields'
- $ref: '#/components/schemas/EsqlRuleRequiredFields'

EsqlRule:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const MINIMUM_LICENSE_FOR_SUPPRESSION = 'platinum' as const;

export const SUPPRESSIBLE_ALERT_RULES: Type[] = [
'threshold',
'esql',
'saved_query',
'query',
'new_terms',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ describe('Alert Suppression Rules', () => {
describe('isSuppressibleAlertRule', () => {
test('should return true for a suppressible rule type', () => {
// Rule types that support alert suppression:
expect(isSuppressibleAlertRule('esql')).toBe(true);
expect(isSuppressibleAlertRule('threshold')).toBe(true);
expect(isSuppressibleAlertRule('saved_query')).toBe(true);
expect(isSuppressibleAlertRule('query')).toBe(true);
Expand All @@ -238,7 +239,6 @@ describe('Alert Suppression Rules', () => {

// Rule types that don't support alert suppression:
expect(isSuppressibleAlertRule('machine_learning')).toBe(false);
expect(isSuppressibleAlertRule('esql')).toBe(false);
});

test('should return false for an unknown rule type', () => {
Expand Down Expand Up @@ -266,6 +266,7 @@ describe('Alert Suppression Rules', () => {
describe('isSuppressionRuleConfiguredWithDuration', () => {
test('should return true for a suppressible rule type', () => {
// Rule types that support alert suppression:
expect(isSuppressionRuleConfiguredWithDuration('esql')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('threshold')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('query')).toBe(true);
Expand All @@ -275,7 +276,6 @@ describe('Alert Suppression Rules', () => {

// Rule types that don't support alert suppression:
expect(isSuppressionRuleConfiguredWithDuration('machine_learning')).toBe(false);
expect(isSuppressionRuleConfiguredWithDuration('esql')).toBe(false);
});

test('should return false for an unknown rule type', () => {
Expand All @@ -288,6 +288,7 @@ describe('Alert Suppression Rules', () => {
describe('isSuppressionRuleConfiguredWithGroupBy', () => {
test('should return true for a suppressible rule type with groupBy', () => {
// Rule types that support alert suppression groupBy:
expect(isSuppressionRuleConfiguredWithGroupBy('esql')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('query')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('threat_match')).toBe(true);
Expand All @@ -296,7 +297,6 @@ describe('Alert Suppression Rules', () => {

// Rule types that don't support alert suppression:
expect(isSuppressionRuleConfiguredWithGroupBy('machine_learning')).toBe(false);
expect(isSuppressionRuleConfiguredWithGroupBy('esql')).toBe(false);
});

test('should return false for a threshold rule type', () => {
Expand All @@ -314,6 +314,7 @@ describe('Alert Suppression Rules', () => {
describe('isSuppressionRuleConfiguredWithMissingFields', () => {
test('should return true for a suppressible rule type with missing fields', () => {
// Rule types that support alert suppression groupBy:
expect(isSuppressionRuleConfiguredWithMissingFields('esql')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('query')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('threat_match')).toBe(true);
Expand All @@ -322,7 +323,6 @@ describe('Alert Suppression Rules', () => {

// Rule types that don't support alert suppression:
expect(isSuppressionRuleConfiguredWithMissingFields('machine_learning')).toBe(false);
expect(isSuppressionRuleConfiguredWithMissingFields('esql')).toBe(false);
});

test('should return false for a threshold rule type', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ export const allowedExperimentalValues = Object.freeze({
*/
disableTimelineSaveTour: false,

/**
* Enables alerts suppression for ES|QL rules
*/
alertSuppressionForEsqlRuleEnabled: false,

/**
* Enables the risk engine privileges route
* and associated callout in the UI
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ Examples:
│ New Terms │ Custom query │ Overview │ Definition │
│ New Terms │ Filters │ Overview │ Definition │
│ ESQL │ ESQL query │ Overview │ Definition │
│ ESQL │ Suppress alerts by │ Overview │ Definition │
│ ESQL │ Suppress alerts for │ Overview │ Definition │
│ ESQL │ If a suppression field is missing │ Overview │ Definition │
```

## Scenarios
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
* 2.0.
*/

import { computeHasMetadataOperator } from './esql_validator';
import { parseEsqlQuery, computeHasMetadataOperator } from './esql_validator';

import { computeIsESQLQueryAggregating } from '@kbn/securitysolution-utils';

jest.mock('@kbn/securitysolution-utils', () => ({ computeIsESQLQueryAggregating: jest.fn() }));

const computeIsESQLQueryAggregatingMock = computeIsESQLQueryAggregating as jest.Mock;

describe('computeHasMetadataOperator', () => {
it('should be false if query does not have operator', () => {
Expand Down Expand Up @@ -44,3 +50,37 @@ describe('computeHasMetadataOperator', () => {
).toBe(true);
});
});

describe('parseEsqlQuery', () => {
it('returns isMissingMetadataOperator true when query is not aggregating and does not have metadata operator', () => {
computeIsESQLQueryAggregatingMock.mockReturnValueOnce(false);

expect(parseEsqlQuery('from test*')).toEqual({
isEsqlQueryAggregating: false,
isMissingMetadataOperator: true,
});
});

it('returns isMissingMetadataOperator false when query is not aggregating and has metadata operator', () => {
computeIsESQLQueryAggregatingMock.mockReturnValueOnce(false);

expect(parseEsqlQuery('from test* metadata _id')).toEqual({
isEsqlQueryAggregating: false,
isMissingMetadataOperator: false,
});
});

it('returns isMissingMetadataOperator false when query is aggregating', () => {
computeIsESQLQueryAggregatingMock.mockReturnValue(true);

expect(parseEsqlQuery('from test*')).toEqual({
isEsqlQueryAggregating: true,
isMissingMetadataOperator: false,
});

expect(parseEsqlQuery('from test* metadata _id')).toEqual({
isEsqlQueryAggregating: true,
isMissingMetadataOperator: false,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
*/

import { isEmpty } from 'lodash';

import type { QueryClient } from '@tanstack/react-query';
import { computeIsESQLQueryAggregating } from '@kbn/securitysolution-utils';

import { KibanaServices } from '../../../common/lib/kibana';
import { securitySolutionQueryClient } from '../../../common/containers/query_client/query_client_provider';

import type { ValidationError, ValidationFunc } from '../../../shared_imports';
import { isEsqlRule } from '../../../../common/detection_engine/utils';
Expand Down Expand Up @@ -48,7 +47,7 @@ export const computeHasMetadataOperator = (esqlQuery: string) => {
export const esqlValidator = async (
...args: Parameters<ValidationFunc>
): Promise<ValidationError<ERROR_CODES> | void | undefined> => {
const [{ value, formData }] = args;
const [{ value, formData, customData }] = args;
const { query: queryValue } = value as FieldValueQueryBar;
const query = queryValue.query as string;
const { ruleType } = formData as DefineStepRule;
Expand All @@ -59,19 +58,19 @@ export const esqlValidator = async (
}

try {
const services = KibanaServices.get();
const queryClient = (customData.value as { queryClient: QueryClient | undefined })?.queryClient;

const isEsqlQueryAggregating = computeIsESQLQueryAggregating(query);
const services = KibanaServices.get();
const { isEsqlQueryAggregating, isMissingMetadataOperator } = parseEsqlQuery(query);

// non-aggregating query which does not have metadata, is not a valid one
if (!isEsqlQueryAggregating && !computeHasMetadataOperator(query)) {
if (isMissingMetadataOperator) {
return {
code: ERROR_CODES.ERR_MISSING_ID_FIELD_FROM_RESULT,
message: i18n.ESQL_VALIDATION_MISSING_ID_IN_QUERY_ERROR,
};
}

const columns = await securitySolutionQueryClient.fetchQuery(
const columns = await queryClient?.fetchQuery(
getEsqlQueryConfig({ esqlQuery: query, search: services.data.search.search })
);

Expand All @@ -92,3 +91,17 @@ export const esqlValidator = async (
return constructValidationError(error);
}
};

/**
* check if esql query valid for Security rule:
* - if it's non aggregation query it must have metadata operator
*/
export const parseEsqlQuery = (query: string) => {
const isEsqlQueryAggregating = computeIsESQLQueryAggregating(query);

return {
isEsqlQueryAggregating,
// non-aggregating query which does not have [metadata], is not a valid one
isMissingMetadataOperator: !isEsqlQueryAggregating && !computeHasMetadataOperator(query),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ describe('description_step', () => {
});

describe('alert suppression', () => {
const ruleTypesWithoutSuppression: Type[] = ['esql', 'machine_learning'];
const ruleTypesWithoutSuppression: Type[] = ['machine_learning'];
const suppressionFields = {
groupByDuration: {
unit: 'm',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { useKibana } from '../../../../common/lib/kibana';
import { useRuleIndices } from '../../../rule_management/logic/use_rule_indices';
import { EsqlAutocomplete } from '../esql_autocomplete';
import { MultiSelectFieldsAutocomplete } from '../multi_select_fields';
import { useInvestigationFields } from '../../hooks/use_investigation_fields';
import { useAllEsqlRuleFields } from '../../hooks';
import { MaxSignals } from '../max_signals';

const CommonUseField = getUseField({ component: Field });
Expand Down Expand Up @@ -133,10 +133,11 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
[getFields]
);

const { investigationFields, isLoading: isInvestigationFieldsLoading } = useInvestigationFields({
esqlQuery: isEsqlRuleValue ? esqlQuery : undefined,
indexPatternsFields: indexPattern.fields,
});
const { fields: investigationFields, isLoading: isInvestigationFieldsLoading } =
useAllEsqlRuleFields({
esqlQuery: isEsqlRuleValue ? esqlQuery : undefined,
indexPatternsFields: indexPattern.fields,
});

return (
<>
Expand Down
Loading

0 comments on commit 6e6b99c

Please sign in to comment.