Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Detections] Adds support for system actions (and cases action) to detection rules #183937

Merged
merged 74 commits into from
Jul 26, 2024

Conversation

dhurley14
Copy link
Contributor

@dhurley14 dhurley14 commented May 21, 2024

Summary

Usually, connectors are used to communicate with external services like PagerDuty or ServiceNow. Automated actions like opening a case or running an OsQuery do not need any configuration. These types of actions are known as "system actions" and represent ways to automate functionality between different kibana features.

This PR allows detection rules to take advantage of system actions, like the cases action, to expedite tasks which our security customers might already undertake, while providing support for future system actions to be added later. You can read more about the cases action here: #153837

case_action_frequency_text

Testing

The cases system action is only available for Platinum licensing. To test all we need is to create a rule and in the Actions tab, select Cases, then turn the rule on and have it generate alerts. When the alerts are generated, a case (or cases) will be created and can be viewed in the Cases page.

cases_system_action_configuration

The configuration options shown in the above screenshot are explained in more detail in this RFC, specifically Diagram 1, but to summarize:

  1. Group by field determines how the alerts are attached to a specific or other cases
  2. Time window is for whenever a case is older than the given time window, and the rule finds more alerts, the case system action will generate a new case after the "time window" period has elapsed.
  3. Reopen - This will reopen the case if the rule discovers alerts after the case has been closed.

The grouping pieces can be better understood via the following example:

If I select agent.type in the group by field for the cases action, and have a rule that generates alerts from packetbeat-* and from auditbeat-* then that singular rule will generate two separate cases. One for alerts grouped by agent.type: packetbeat and another case for auditbeat

cases_action_grouping_example

And we can still use system actions in conjunction with other action connectors.

Checklist

Delete any items that are not applicable to this PR.

  • Documentation was added for features that require explanation or tutorials: Security Solution docs ticket here
  • Unit or functional tests were updated or added to match the most common scenarios (Todo)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

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
Impact of failing system action on other actions associated with the rule Low High Ensure testing system actions and third party connectors, mock a failure in cases api endpoints

@dhurley14 dhurley14 added ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment ci:project-deploy-security Create a Security Serverless Project ci:project-redeploy Always create a new Cloud project labels Jun 27, 2024
@dhurley14 dhurley14 self-assigned this Jun 27, 2024
@dhurley14
Copy link
Contributor Author

/ci

@dhurley14 dhurley14 added the release_note:skip Skip the PR/issue when compiling release notes label Jun 27, 2024
@@ -479,7 +479,6 @@ components:
$ref: '#/components/schemas/RuleActionFrequency'
required:
- action_type_id
- group
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any questions on this, I pulled this from the system actions RFC:

All rules APIs will not accept the systemActions in their requests. The system will deduct if an action is a system action automatically using the actions’ client. As system actions are not allowed to have a group, the property will be optional (at the moment it is required) in all APIs. The APIs will convert the actions to the schema needed by the rules client. Finally, the APIs will merge the system actions with the default actions before sending the response back to the client.

Ref: https://docs.google.com/document/d/1mYStNLxrafnyxR6v_uB4ONw_iFHTg1pmE3EAQYgKFSY/edit?pli=1

@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 18, 2024

⏳ Build in-progress

History

cc @dhurley14

@dhurley14
Copy link
Contributor Author

dhurley14 commented Jul 19, 2024

Here is a list of test cases I tried manually. All passed:

  1. Can I update a rule’s query without modifying or deleting system action? ✅
  2. Can I update a single rule’s system action?✅
  3. Can I duplicate a single rule and have the system action be present?✅
  4. Can I update a single rule’s action without modifying / deleting the system action?✅
  5. Can I create a rule with two non-system actions?✅
  6. Can I create a rule with two non-system actions and the cases action?✅
  7. Can I update a rule's non-system action?✅
  8. Can I use the bulk add action feature to add a system action?✅
  9. Can I use the bulk add action feature to add a non-system action and ensure it did not remove a system action already present on a rule?✅
  10. Can I use the bulk add action feature to add the cases system action and ensure it did not remove a non-system action from a rule?✅
  11. Can I use the bulk add action feature to add the cases action and overwrite other rules actions?✅
  12. Can I use the bulk add action feature to add a non-system action and overwrite the other rules actions, including overwriting a system action?✅
  13. Can I import and export a rule with system actions?✅
  14. Will a case be created if an alert does not have a field?✅ (displays 'unknown' as the group by value and attaches the correct number of alerts)

@vitaliidm vitaliidm self-requested a review July 22, 2024 09:03
Copy link
Contributor

@jpdjere jpdjere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dhurley14. Thanks for addressing my points. Rechecked the bulk adding of system actions and everything seems to be working as expected now 👍

Left a couple of nits, but approving ✅

(I did cause a conflict in x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/internal_rule_to_api_response.ts when I merged #188631, sorry about that)

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this feature and addressing comments.

Approving it, only one test to add left: #183937 (comment)

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a high-level review and testing. LGTM! I noticed that the rule description does not show how the system actions are executed. System actions always work with alert summaries and they are executed on each rule run if the rule matches (triggers). Should we add something to describe that for all system actions?

Screenshot 2024-07-25 at 5 17 34 PM

In the stack management or the o11y we show

Screenshot 2024-07-25 at 5 15 03 PM

Also, unrelated to the PR, I noticed that the ITSM connector is not working as expected. cc @js-jankisalvi

Screenshot 2024-07-25 at 5 11 52 PM

);
};

export const getCases = async (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: There is already a function to find cases in x-pack/test/cases_api_integration/common/lib/api/index.ts? What do you think of using that instead? Feel free to extend it to match your needs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did investigate this however I could not find a clean way to go about doing this. The options all seemed to introduce too much complexity into the findCases function for a single security solution test case. The problems stem from the use of the auth parameter in the findCases function, whereas we use a supertest that has the auth added to the header to support both ESS and serverless test environments. To update the findCases function to support our use-case would be introducing more maintenance overhead than it was worth. At least that's what I found to be the situation. Thank you for the review!

x-pack/test/cases_api_integration/common/lib/api/case.ts Outdated Show resolved Hide resolved
@rylnd rylnd removed their request for review July 25, 2024 19:27
@js-jankisalvi
Copy link
Contributor

js-jankisalvi commented Jul 26, 2024

Also, unrelated to the PR, I noticed that the ITSM connector is not working as expected. cc @js-jankisalvi

@cnasikas Verified it happens in main as well. Created an issue.

@dhurley14 dhurley14 enabled auto-merge (squash) July 26, 2024 15:53
@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 26, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #20 / serverless search UI Serverless Playground Overview setup Page with existing indices can select index from dropdown and load chat page

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 20.4MB 20.4MB +3.0KB

History

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

cc @dhurley14

@dhurley14 dhurley14 merged commit d749305 into elastic:main Jul 26, 2024
44 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment ci:cloud-redeploy Always create a new Cloud deployment ci:project-deploy-security Create a Security Serverless Project Feature:Rule Actions Security Solution Rule Actions feature release_note:feature Makes this part of the condensed release notes review Team:Detection Engine Security Solution Detection Engine Area v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.