[Security Solution][Detection Engine] adds alert suppression to Indicator Match rule #174241

vitaliidm · 2024-01-04T10:12:16Z

Summary

addresses https://github.com/elastic/security-team/issues/7773 Epic
addresses https://github.com/elastic/security-team/issues/8360

Alert suppression for these rule types is hidden behind feature branch

In this PR implemented:

schema changes: allowing alert_suppression object in Indicator match rule type. alert_suppression is identical to existing one for query rule
UI changes
Cypress tests
BE implementation
FTR tests

Enabling feature flags

alertSuppressionForIndicatorMatchRuleEnabled

Tech implementation details

Alert candidates for IM rule deduplicated first, by searching in existing alerts matched ids.
Once retrieved, alert candidates filtered further, to determine whether they been already suppressed.
It's done by checking each alert candidate suppression time boundaries. If suppression ends earlier than existing alert suppression with the same instance id, alert candidate is removed.

The rest of alert candidates are getting suppressed in memory and either new alerts created or existing updated.
The max limit of created and suppressed alerts is set to 5 * max_signals, which would allow to capture additional threats, should rule execution's alerts number reach max_signals

UI changes

Suppression components in IM rule are identical to Custom Query's

UI changes

Checklist

Functional changes are hidden behind a feature flag

Feature flag alertSuppressionForThresholdRuleEnabled
Functional changes are covered with a test plan and automated tests.

Test plan PR

Stability of new and changed tests is verified using the Flaky Test Runner.

[FTR ESS & Serverless tests] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4972
[Cypress ESS] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4970
[Cypress Serverless] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4971
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.
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 IM rule, the one that used for Query rule.

    alert_suppression:
      $ref: './common_attributes.schema.yaml#/components/schemas/AlertSuppression'

where

    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

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#4715

## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

…sts fro IM suppression (#174344) ## Summary Adds tests by test plan **Scenario: Create rule with per rule execution suppression** Given rule create page When user select rule type And user adds suppress fields And user selects on rule execution suppression only And user saves rule Then on rule details page suppress by fields should be displayed And suppression on rule execution should be rendered **Scenario: Create rule with time interval suppression** Given rule create page When user select rule type And user adds suppress fields And user selects time interval suppression option And user selects do not suppress missing fields And user saves rule Then on rule details page suppress by fields should be displayed And time interval suppression should be displayed **Scenario: Edit rule with suppression** Given rule configured with suppression on rule interval When user edits rule And changes suppression from time interval to rule execution suppression Then on rule details page suppress by fields should be displayed And per rule execution suppression should be displayed **Scenario: Edit rule without suppression** Given rule without suppression When user edits rule And user adds suppress fields And user selects time interval suppression option And user saves rule Then on rule details page suppress by fields should be displayed And time interval suppression should be displayed **Scenario: Rule details with suppression** Given rule configured with suppression time interval When user views rule details page Then on rule details page suppress by fields should be displayed And time interval suppression should be displayed ### **License tests** **Scenario: Create rule with rule execution suppression on basic license (ESS only)** Given rule create page When user select rule type Then user sees suppression options disabled And upselling message displayed **Scenario: Create rule with rule execution suppression on Essentials tier (Serverless only)** Given rule create page When user select rule type And user adds suppress fields And user selects on rule execution suppression only And user saves rule Then on rule details page suppress by fields should be displayed And suppression on rule execution should be rendered **Scenario: Rule details with suppression on basic license (ESS only)** Given rule configured with suppression time interval When user views rule details page Then on rule details page suppress by fields should be displayed And time interval suppression should be displayed And upselling warning that suppression is not applied should be rendered on details labels

## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

...s/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts

x-pack/test/security_solution_cypress/cypress/objects/rule.ts

## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] 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/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

MadameSheema

security-engineering-productivity changes LGTM. Thanks!! :)

e40pud

Looks great! I left a few minor comments and questions. I will need a bit more time to do local testing, but in general code LGTM

x-pack/plugins/rule_registry/common/schemas/8.13.0/index.ts

e40pud · 2024-02-02T12:54:51Z

x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts

+        suppressedAlerts.push(alert);
+        return false;
+      } else {
+        idsMap[instanceId] = { count: suppressionDocsCount, suppressionEnd };


Just trying to understand what instanceId == null means and in what case that can happen? Also, what does that mean in terms of updating count and suppressionEnd values?

Based on lines 177-184, we would leave such alerts as it is, meaning we are not going to do suppression for them?

the list of alerts get sorted first by supression start https://github.com/elastic/kibana/pull/174241/files#diff-fa438b70a9910ed86557d3e5bc51ed29dfefbcf746d86d8fd08c203ab7251f2eR155

Afterwards, we are traversing array and suppressing alerts based on instance_id.
when particular instance_id is not yet in map(instanceId == null), that is the first alert, with the smallest suppression_end(it is equals to suppression_start at this moment). We take it as a basis, value of suppression_end and docs_count.
In next iteration, if we encounter alert with the same instanceId, we increase docs_count(means we count that alert as suppressed) and changing suppression_end, so the correct interval of suppression is recorded

e40pud · 2024-02-02T13:03:37Z

x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts

@@ -257,7 +406,7 @@ export const createPersistenceRuleTypeWrapper: CreatePersistenceRuleTypeWrapper
                          },
                          {
                            terms: {
-                              [ALERT_INSTANCE_ID]: alerts.map(
+                              [ALERT_INSTANCE_ID]: filteredDuplicates.map(
                                (alert) => alert._source['kibana.alert.instance.id']


should we use ALERT_INSTANCE_ID here as well?

e40pud · 2024-02-02T13:09:39Z

x-pack/plugins/rule_registry/server/utils/create_persistence_rule_type_wrapper.ts

+ * alerts returned from BE have date type coerced to ISO strings
+ */
+export type BackendAlertWithSuppressionFields870<T> = Omit<
+  AlertWithSuppressionFields870<T>,


why don't we use 8.13. version here?

@e40pud , thanks for review. Have addressed the feedback

there is an explanation that was there from the initial implementation.
I moved it to the redefined type, so it is more visible
e4239b6#diff-fa438b70a9910ed86557d3e5bc51ed29dfefbcf746d86d8fd08c203ab7251f2eL439-L441

...ins/security_solution/server/lib/detection_engine/rule_types/utils/wrap_suppressed_alerts.ts

...ns/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts

WafaaNasr · 2024-02-02T14:43:13Z

...n/server/lib/detection_engine/rule_types/utils/search_after_bulk_create_suppressed_alerts.ts

+    enrichedEvents,
+    toReturn,
+  }) => {
+    // max signals for suppression includes suppressed and created alerts


I think this function could be more reusable if it just deals with the suppression part and lets the caller handle alert creation. The utility currently takes many parameters for searchAfterAndBulkCreateFactory, which is also called in the search_after_bulk_create.ts.

By including only the suppression-specific parameters, we can use this utility in various executors. Additionally, certain parts of this utility's code can be shared with the search_after_bulk_create.

Suppression logic is tightly interconnected with alert creation.
In this executor bulkCreate is called to create non suppressed alerts with missing fields, and alerts are suppressed and created in bulkCreateWithSuppression. Then there is a logic specific to alert truncation, that can't be handled out of scope of alert creation.
So, code in this executor is specific for alert suppression only. All common logic between searchAfterAndBulkCreateSuppressedAlerts and searchAfterAndBulkCreate is implemented within searchAfterAndBulkCreateFactory. While, suppression or non-suppression alert creation handled in corresponding executor.

In future, executor might be extracted as a separate utility, when it will be needed only for suppression/creation, without exhaustive search. For example, for EQL case

@WafaaNasr it sounds like this could be very helpful to allow us to better unit test and resuse the functionality. Could we create a ticket to track this improvement and perhaps include it with the EQL alert suppression work?

Thanks Vitalii and Yara!
We talked about this matter and agreed that Vitalli will focus on extracting the logic here to make it reusable for other rule types in a different PR

Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

pmuellr

ResponseOps changes LGTM

e40pud

Tested locally and did not find any issues. LGTM!

kibana-ci · 2024-02-05T10:55:57Z

💚 Build Succeeded

Buildkite Build
Commit: ffbd021

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	4951	4952	+1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`ruleRegistry`	239	243	+4

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	11.4MB	11.4MB	+1.7KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	69.7KB	69.9KB	+135.0B

Unknown metric groups

API count

id	before	after	diff
`ruleRegistry`	268	272	+4

References to deprecated APIs

id	before	after	diff
`securitySolution`	519	521	+2

History

💛 Build #191124 was flaky a0122e9
💛 Build #191038 was flaky 55dde08
💚 Build #190842 succeeded 3aa86a6
💛 Build #190131 was flaky ee91214
💔 Build #189974 failed bb86f93
💛 Build #189823 was flaky adf1030

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @vitaliidm

WafaaNasr

LGTM!
Thanks Vitalii for addressing the comments!

…ator Match rule (elastic#174241) ## Summary - addresses elastic/security-team#7773 Epic - addresses elastic/security-team#8360 Alert suppression for these rule types is hidden behind feature branch In this PR implemented: - schema changes: allowing `alert_suppression` object in Indicator match rule type. `alert_suppression` is identical to existing one for query rule - UI changes - Cypress tests - BE implementation - FTR tests Enabling feature flags - `alertSuppressionForIndicatorMatchRuleEnabled` ### Tech implementation details Alert candidates for IM rule deduplicated first, by searching in existing alerts matched ids. Once retrieved, alert candidates filtered further, to determine whether they been already suppressed. It's done by checking each alert candidate suppression time boundaries. If suppression ends earlier than existing alert suppression with the same instance id, alert candidate is removed. The rest of alert candidates are getting suppressed in memory and either new alerts created or existing updated. The max limit of created and suppressed alerts is set to `5 * max_signals`, which would allow to capture additional threats, should rule execution's alerts number reach max_signals ### UI changes Suppression components in IM rule are identical to Custom Query's ![localhost_5601_kbn_app_security_rules_create (2)](https://github.com/elastic/kibana/assets/92328789/b79db59a-9369-4ef0-af4a-c0ca0bb3d25d) ### UI changes ### Checklist - [x] Functional changes are hidden behind a feature flag Feature flag `alertSuppressionForThresholdRuleEnabled` - [x] Functional changes are covered with a test plan and automated tests. [Test plan PR](elastic/security-team#8390) - [x] Stability of new and changed tests is verified using the [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner). [FTR ESS & Serverless tests] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4972 [Cypress ESS] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4970 [Cypress Serverless] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4971 - [ ] 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 IM rule, the one that used for Query rule. ```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#4715 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Wafaa Nasr <wafaa.nasr@elastic.co> Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

…order of search to asc (#176321) ## Summary Sets search of documents for IM rule type from `desc` to `asc` when suppression is enabled. Also would allow to fix corner cases around [alert suppression](#174241). Alert suppression in IM rule relies on correct suppression time boundaries to correctly deduplicate earlier suppressed alerts. I.e, if document start suppression time(document timestamp) falls within suppression boundaries, it means, alert was already suppressed. So, we can exclude it from suppression as already suppressed and not to count it twice. But because documents for IM rule are searched in reverse order, it is possible, while processing a second page of results, to falsely count alert as already suppressed and discard it from suppressed count. That's because its timestamp is older than document's timestamp from the first page. Newly added test failed only for code execution path, when number of events is greater than number of threats. It is because, events are split in chunks by 9,000 first. So if reverse order in that case would cause alert from next batches to be dropped as already suppressed Setting `asc` can potentially affect IM rule performance, when events need to be searched first and rule is configured with the large look-back time. That's why new order is set to tech preview alert suppression feature only --------- Co-authored-by: Ryland Herrick <ryalnd@gmail.com>

…ator Match rule (elastic#174241) ## Summary - addresses elastic/security-team#7773 Epic - addresses elastic/security-team#8360 Alert suppression for these rule types is hidden behind feature branch In this PR implemented: - schema changes: allowing `alert_suppression` object in Indicator match rule type. `alert_suppression` is identical to existing one for query rule - UI changes - Cypress tests - BE implementation - FTR tests Enabling feature flags - `alertSuppressionForIndicatorMatchRuleEnabled` ### Tech implementation details Alert candidates for IM rule deduplicated first, by searching in existing alerts matched ids. Once retrieved, alert candidates filtered further, to determine whether they been already suppressed. It's done by checking each alert candidate suppression time boundaries. If suppression ends earlier than existing alert suppression with the same instance id, alert candidate is removed. The rest of alert candidates are getting suppressed in memory and either new alerts created or existing updated. The max limit of created and suppressed alerts is set to `5 * max_signals`, which would allow to capture additional threats, should rule execution's alerts number reach max_signals ### UI changes Suppression components in IM rule are identical to Custom Query's ![localhost_5601_kbn_app_security_rules_create (2)](https://github.com/elastic/kibana/assets/92328789/b79db59a-9369-4ef0-af4a-c0ca0bb3d25d) ### UI changes ### Checklist - [x] Functional changes are hidden behind a feature flag Feature flag `alertSuppressionForThresholdRuleEnabled` - [x] Functional changes are covered with a test plan and automated tests. [Test plan PR](elastic/security-team#8390) - [x] Stability of new and changed tests is verified using the [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner). [FTR ESS & Serverless tests] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4972 [Cypress ESS] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4970 [Cypress Serverless] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4971 - [ ] 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 IM rule, the one that used for Query rule. ```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#4715 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Wafaa Nasr <wafaa.nasr@elastic.co> Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

…order of search to asc (elastic#176321) ## Summary Sets search of documents for IM rule type from `desc` to `asc` when suppression is enabled. Also would allow to fix corner cases around [alert suppression](elastic#174241). Alert suppression in IM rule relies on correct suppression time boundaries to correctly deduplicate earlier suppressed alerts. I.e, if document start suppression time(document timestamp) falls within suppression boundaries, it means, alert was already suppressed. So, we can exclude it from suppression as already suppressed and not to count it twice. But because documents for IM rule are searched in reverse order, it is possible, while processing a second page of results, to falsely count alert as already suppressed and discard it from suppressed count. That's because its timestamp is older than document's timestamp from the first page. Newly added test failed only for code execution path, when number of events is greater than number of threats. It is because, events are split in chunks by 9,000 first. So if reverse order in that case would cause alert from next batches to be dropped as already suppressed Setting `asc` can potentially affect IM rule performance, when events need to be searched first and rule is configured with the large look-back time. That's why new order is set to tech preview alert suppression feature only --------- Co-authored-by: Ryland Herrick <ryalnd@gmail.com>

…ator Match rule (elastic#174241) ## Summary - addresses elastic/security-team#7773 Epic - addresses elastic/security-team#8360 Alert suppression for these rule types is hidden behind feature branch In this PR implemented: - schema changes: allowing `alert_suppression` object in Indicator match rule type. `alert_suppression` is identical to existing one for query rule - UI changes - Cypress tests - BE implementation - FTR tests Enabling feature flags - `alertSuppressionForIndicatorMatchRuleEnabled` ### Tech implementation details Alert candidates for IM rule deduplicated first, by searching in existing alerts matched ids. Once retrieved, alert candidates filtered further, to determine whether they been already suppressed. It's done by checking each alert candidate suppression time boundaries. If suppression ends earlier than existing alert suppression with the same instance id, alert candidate is removed. The rest of alert candidates are getting suppressed in memory and either new alerts created or existing updated. The max limit of created and suppressed alerts is set to `5 * max_signals`, which would allow to capture additional threats, should rule execution's alerts number reach max_signals ### UI changes Suppression components in IM rule are identical to Custom Query's ![localhost_5601_kbn_app_security_rules_create (2)](https://github.com/elastic/kibana/assets/92328789/b79db59a-9369-4ef0-af4a-c0ca0bb3d25d) ### UI changes ### Checklist - [x] Functional changes are hidden behind a feature flag Feature flag `alertSuppressionForThresholdRuleEnabled` - [x] Functional changes are covered with a test plan and automated tests. [Test plan PR](elastic/security-team#8390) - [x] Stability of new and changed tests is verified using the [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner). [FTR ESS & Serverless tests] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4972 [Cypress ESS] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4970 [Cypress Serverless] https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4971 - [ ] 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 IM rule, the one that used for Query rule. ```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#4715 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Wafaa Nasr <wafaa.nasr@elastic.co> Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

…order of search to asc (elastic#176321) ## Summary Sets search of documents for IM rule type from `desc` to `asc` when suppression is enabled. Also would allow to fix corner cases around [alert suppression](elastic#174241). Alert suppression in IM rule relies on correct suppression time boundaries to correctly deduplicate earlier suppressed alerts. I.e, if document start suppression time(document timestamp) falls within suppression boundaries, it means, alert was already suppressed. So, we can exclude it from suppression as already suppressed and not to count it twice. But because documents for IM rule are searched in reverse order, it is possible, while processing a second page of results, to falsely count alert as already suppressed and discard it from suppressed count. That's because its timestamp is older than document's timestamp from the first page. Newly added test failed only for code execution path, when number of events is greater than number of threats. It is because, events are split in chunks by 9,000 first. So if reverse order in that case would cause alert from next batches to be dropped as already suppressed Setting `asc` can potentially affect IM rule performance, when events need to be searched first and rule is configured with the large look-back time. That's why new order is set to tech preview alert suppression feature only --------- Co-authored-by: Ryland Herrick <ryalnd@gmail.com>

vitaliidm added 3 commits January 4, 2024 10:09

Merge branch 'main' into security/alert-suppression-im-eql

89df703

vitaliidm self-assigned this Jan 4, 2024

vitaliidm added 14 commits January 4, 2024 11:06

Update rule_schemas.gen.ts

4f36323

Merge branch 'main' into security/alert-suppression-im-eql

cc0ae5c

Update rule_definition_section.tsx

2dfa47c

Merge branch 'main' into security/alert-suppression-im-eql

1c07513

Merge branch 'main' into security/alert-suppression-im-eql

72dd1c0

Merge branch 'main' into security/alert-suppression-im-eql

2273589

Merge branch 'main' into security/alert-suppression-im-eql

88fb535

Merge branch 'main' into security/alert-suppression-im-eql

912bc02

Update mappings.json

8861a8b

Merge branch 'main' into security/alert-suppression-im-eql

54a698b

Merge branch 'main' into security/alert-suppression-im-eql

b929e9d

Merge branch 'main' into security/alert-suppression-im-eql

5a6624c

WafaaNasr reviewed Jan 15, 2024

View reviewed changes

...s/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts Outdated Show resolved Hide resolved

x-pack/test/security_solution_cypress/cypress/objects/rule.ts Show resolved Hide resolved

vitaliidm and others added 3 commits January 16, 2024 17:19

Update indicator_match_rule.cy.ts

0def0da

Update schema.tsx

48334dd

vitaliidm changed the title ~~[Security Solution][Detection Engine] adds alert suppression to IM and EQL rules~~ [Security Solution][Detection Engine] adds alert suppression to Indicator Match rule Jan 17, 2024

Merge branch 'main' into security/alert-suppression-im-eql

c2713f0

vitaliidm added 2 commits February 2, 2024 12:42

address PR feedback

f0ad4f2

Merge branch 'main' into security/alert-suppression-im-eql

55dde08

MadameSheema approved these changes Feb 2, 2024

View reviewed changes

e40pud reviewed Feb 2, 2024

View reviewed changes

WafaaNasr reviewed Feb 2, 2024

View reviewed changes

vitaliidm and others added 3 commits February 2, 2024 15:19

Update x-pack/plugins/rule_registry/common/schemas/8.13.0/index.ts

c3e6fb9

Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

Update x-pack/plugins/rule_registry/common/schemas/8.13.0/index.ts

73d482c

Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>

PR feedback

e4239b6

vitaliidm requested a review from e40pud February 2, 2024 15:39

PR feedback

a0122e9

vitaliidm requested a review from WafaaNasr February 2, 2024 16:41

pmuellr approved these changes Feb 2, 2024

View reviewed changes

Merge branch 'main' into security/alert-suppression-im-eql

ffbd021

e40pud approved these changes Feb 5, 2024

View reviewed changes

WafaaNasr approved these changes Feb 5, 2024

View reviewed changes

vitaliidm merged commit 6d5a485 into main Feb 5, 2024
37 checks passed

vitaliidm deleted the security/alert-suppression-im-eql branch February 5, 2024 15:54

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Feb 5, 2024

vitaliidm mentioned this pull request Feb 6, 2024

[Security Solution][Detection Engine] sets Indicator match rule sort order of search to asc #176321

Merged

Mikaayenson mentioned this pull request Aug 14, 2024

[FR] Add Alert Suppression for Addtional Rule Types elastic/detection-rules#3986

Merged

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detection Engine] adds alert suppression to Indicator Match rule #174241

[Security Solution][Detection Engine] adds alert suppression to Indicator Match rule #174241

vitaliidm commented Jan 4, 2024 •

edited

Loading

MadameSheema left a comment

e40pud left a comment

e40pud Feb 2, 2024

vitaliidm Feb 2, 2024

e40pud Feb 2, 2024

e40pud Feb 2, 2024

vitaliidm Feb 2, 2024

WafaaNasr Feb 2, 2024

vitaliidm Feb 2, 2024 •

edited

Loading

yctercero Feb 5, 2024

WafaaNasr Feb 5, 2024

pmuellr left a comment

e40pud left a comment

kibana-ci commented Feb 5, 2024

API count

References to deprecated APIs

WafaaNasr left a comment

[Security Solution][Detection Engine] adds alert suppression to Indicator Match rule #174241

[Security Solution][Detection Engine] adds alert suppression to Indicator Match rule #174241

Conversation

vitaliidm commented Jan 4, 2024 • edited Loading

Summary

Tech implementation details

UI changes

UI changes

Checklist

MadameSheema left a comment

Choose a reason for hiding this comment

e40pud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaliidm Feb 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

e40pud left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 5, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

References to deprecated APIs

History

WafaaNasr left a comment

Choose a reason for hiding this comment

vitaliidm commented Jan 4, 2024 •

edited

Loading

vitaliidm Feb 2, 2024 •

edited

Loading