Skip to content

Commit

Permalink
[Security Solution][Detection Engine] adds alert suppression for New …
Browse files Browse the repository at this point in the history
…Terms rule type (#178294)

## Summary

 - addresses elastic/security-team#8824
 - adds alert suppression for new terms rule type
- fixes `getOpenAlerts` test function, which returned closed alerts as
well
 
### UI

<img width="2294" alt="Screenshot 2024-04-02 at 12 53 26"
src="https://github.com/elastic/kibana/assets/92328789/8398fba4-a06c-464b-87ef-1c5d5a18e37f">
<img width="1651" alt="Screenshot 2024-04-02 at 12 53 46"
src="https://github.com/elastic/kibana/assets/92328789/971ec0da-c1d9-4c96-a4af-7cc8dfae52a4">



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

  Feature flag `alertSuppressionForNewTermsRuleEnabled`

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

  Test plan: elastic/security-team#9045

- [x] Stability of new and changed tests is verified using the [Flaky
Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner).

Cypress ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5547
Cypress Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5548

FTR ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5596
FTR Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5597

- [ ] 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 New terms rule, the
one that used for Query 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#5030
  • Loading branch information
vitaliidm authored Apr 9, 2024
1 parent 42e92d8 commit 52cfdd6
Show file tree
Hide file tree
Showing 65 changed files with 4,069 additions and 493 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,8 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
suppressionWindow,
enrichAlerts,
currentTimeOverride,
isRuleExecutionOnly
isRuleExecutionOnly,
maxAlerts
) => {
const ruleDataClientWriter = await ruleDataClient.getWriter({
namespace: options.spaceId,
Expand All @@ -376,6 +377,8 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
const writeAlerts =
ruleDataClient.isWriteEnabled() && options.services.shouldWriteAlerts();

let alertsWereTruncated = false;

if (writeAlerts && alerts.length > 0) {
const suppressionWindowStart = dateMath.parse(suppressionWindow, {
forceNow: currentTimeOverride,
Expand All @@ -392,7 +395,12 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
});

if (filteredDuplicates.length === 0) {
return { createdAlerts: [], errors: {}, suppressedAlerts: [] };
return {
createdAlerts: [],
errors: {},
suppressedAlerts: [],
alertsWereTruncated,
};
}

const suppressionAlertSearchRequest = {
Expand Down Expand Up @@ -473,7 +481,12 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
});

if (nonSuppressedAlerts.length === 0) {
return { createdAlerts: [], errors: {}, suppressedAlerts: [] };
return {
createdAlerts: [],
errors: {},
suppressedAlerts: [],
alertsWereTruncated,
};
}

const { alertCandidates, suppressedAlerts: suppressedInMemoryAlerts } =
Expand Down Expand Up @@ -535,6 +548,11 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
}
}

if (maxAlerts && enrichedAlerts.length > maxAlerts) {
enrichedAlerts.length = maxAlerts;
alertsWereTruncated = true;
}

const augmentedAlerts = augmentAlerts({
alerts: enrichedAlerts,
options,
Expand All @@ -548,7 +566,12 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
});

if (bulkResponse == null) {
return { createdAlerts: [], errors: {}, suppressedAlerts: [] };
return {
createdAlerts: [],
errors: {},
suppressedAlerts: [],
alertsWereTruncated: false,
};
}

const createdAlerts = augmentedAlerts
Expand Down Expand Up @@ -594,10 +617,16 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
createdAlerts,
suppressedAlerts: [...duplicateAlerts, ...suppressedInMemoryAlerts],
errors: errorAggregator(bulkResponse.body, [409]),
alertsWereTruncated,
};
} else {
logger.debug('Writing is disabled.');
return { createdAlerts: [], errors: {}, suppressedAlerts: [] };
return {
createdAlerts: [],
errors: {},
suppressedAlerts: [],
alertsWereTruncated: false,
};
}
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ export type SuppressedAlertService = <T extends SuppressionFieldsLatest>(
params: { spaceId: string }
) => Promise<Array<{ _id: string; _source: T }>>,
currentTimeOverride?: Date,
isRuleExecutionOnly?: boolean
isRuleExecutionOnly?: boolean,
maxAlerts?: number
) => Promise<SuppressedAlertServiceResult<T>>;

export interface SuppressedAlertServiceResult<T>
extends Omit<PersistenceAlertServiceResult<T>, 'alertsWereTruncated'> {
export interface SuppressedAlertServiceResult<T> extends PersistenceAlertServiceResult<T> {
suppressedAlerts: Array<{ _id: string; _source: T }>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getListArrayMock } from '../../../../detection_engine/schemas/types/lis
import {
getCreateEsqlRulesSchemaMock,
getCreateMachineLearningRulesSchemaMock,
getCreateNewTermsRulesSchemaMock,
getCreateRulesSchemaMock,
getCreateRulesSchemaMockWithDataView,
getCreateSavedQueryRulesSchemaMock,
Expand Down Expand Up @@ -1267,6 +1268,7 @@ describe('rules schema', () => {
{ ruleType: 'threat_match', ruleMock: getCreateThreatMatchRulesSchemaMock() },
{ ruleType: 'query', ruleMock: getCreateRulesSchemaMock() },
{ ruleType: 'saved_query', ruleMock: getCreateSavedQueryRulesSchemaMock() },
{ ruleType: 'new_terms', ruleMock: getCreateNewTermsRulesSchemaMock() },
];

cases.forEach(({ ruleType, ruleMock }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ export const NewTermsRuleOptionalFields = z.object({
index: IndexPatternArray.optional(),
data_view_id: DataViewId.optional(),
filters: RuleFilterArray.optional(),
alert_suppression: AlertSuppression.optional(),
});

export type NewTermsRuleDefaultableFields = z.infer<typeof NewTermsRuleDefaultableFields>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,8 @@ components:
$ref: './common_attributes.schema.yaml#/components/schemas/DataViewId'
filters:
$ref: './common_attributes.schema.yaml#/components/schemas/RuleFilterArray'
alert_suppression:
$ref: './common_attributes.schema.yaml#/components/schemas/AlertSuppression'

NewTermsRuleDefaultableFields:
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,6 @@ export const SUPPRESSIBLE_ALERT_RULES: Type[] = [
'threshold',
'saved_query',
'query',
'new_terms',
'threat_match',
];
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,11 @@ describe('Alert Suppression Rules', () => {
expect(isSuppressibleAlertRule('saved_query')).toBe(true);
expect(isSuppressibleAlertRule('query')).toBe(true);
expect(isSuppressibleAlertRule('threat_match')).toBe(true);
expect(isSuppressibleAlertRule('new_terms')).toBe(true);

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

Expand All @@ -253,11 +253,11 @@ describe('Alert Suppression Rules', () => {
expect(isSuppressionRuleConfiguredWithDuration('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('query')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('threat_match')).toBe(true);
expect(isSuppressionRuleConfiguredWithDuration('new_terms')).toBe(true);

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

Expand All @@ -274,11 +274,11 @@ describe('Alert Suppression Rules', () => {
expect(isSuppressionRuleConfiguredWithGroupBy('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('query')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('threat_match')).toBe(true);
expect(isSuppressionRuleConfiguredWithGroupBy('new_terms')).toBe(true);

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

Expand All @@ -300,11 +300,11 @@ describe('Alert Suppression Rules', () => {
expect(isSuppressionRuleConfiguredWithMissingFields('saved_query')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('query')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('threat_match')).toBe(true);
expect(isSuppressionRuleConfiguredWithMissingFields('new_terms')).toBe(true);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ export const allowedExperimentalValues = Object.freeze({
*/
riskEnginePrivilegesRouteEnabled: true,

/**
* Enables alerts suppression for new terms rules
*/
alertSuppressionForNewTermsRuleEnabled: false,

/**
* Enables experimental Experimental S1 integration data to be available in Analyzer
*/
Expand Down
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[] = ['eql', 'esql', 'machine_learning', 'new_terms'];
const ruleTypesWithoutSuppression: Type[] = ['eql', 'esql', 'machine_learning'];
const suppressionFields = {
groupByDuration: {
unit: 'm',
Expand Down
Loading

0 comments on commit 52cfdd6

Please sign in to comment.