[RAC][Security Solution] Adds migration to new SecuritySolution rule types #112113

madirey · 2021-09-14T14:26:07Z

Summary

This PR adds a migration for 8.0 to the new security solution rule types.

Changes include

Adds a new @kbn/securitysolution-rules package to share rules data and utilities between plugins
Adds a migration in the alerting plugin to split the siem.signals rule type into 6 distinct DE rule types
Adds flag to disable writer cache in ruleDataClient (for ease of testing)
Replaces most occurrences of signal.* alert fields with kibana.alert.* equivalents
Adds AAD index aliases
Fixes a couple of Threshold Rule bugs
Flattens original_event fields
Removes tests that are no longer applicable

To be addressed later

Skipped tests
Backwards compatibility
Dupe mitigation
Update fields in x-pack/plugins/security_solution/public/common/components/event_details/reason.tsx and x-pack/plugins/security_solution/public/detections/components/alerts_info/query.dsl.ts and x-pack/plugins/timelines/public/hooks/use_add_to_case.ts (fix add to case)
Fix alert actions (action migrations + unflatten fields for Mustache)

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

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

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-type-threshold

…igration

…rules-migration

kibanamachine · 2021-10-26T18:03:30Z

💚 Build Succeeded

Metrics [docs]

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
`@kbn/rule-data-utils`	71	72	+1
`@kbn/securitysolution-rules`	-	21	+21
`ruleRegistry`	128	127	-1
total			+21

Async chunks

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

id	before	after	diff
`observability`	385.6KB	385.7KB	+57.0B
`securitySolution`	4.5MB	4.5MB	+976.0B
`timelines`	241.4KB	242.0KB	+674.0B
total			+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
`apm`	29.3KB	29.4KB	+57.0B
`infra`	90.6KB	90.7KB	+57.0B
`timelines`	157.3KB	157.3KB	+14.0B
`uptime`	24.9KB	25.0KB	+57.0B
total			+185.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/rule-data-utils`	74	75	+1
`@kbn/securitysolution-rules`	-	23	+23
`ruleRegistry`	151	153	+2
total			+26

History

💔 Build #1658 failed 595e979
💔 Build #1644 failed b62e83e
💔 Build #1608 failed 9f2a6ca
💔 Build #1520 failed d4bc86c
💔 Build #1117 failed a68d386

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

michaelolo24 · 2021-10-26T20:07:15Z

x-pack/plugins/security_solution/cypress/integration/detection_alerts/cti_enrichments.spec.ts

@@ -55,7 +55,7 @@ describe('CTI Enrichment', () => {
    goToRuleDetails();
  });

-  it('Displays enrichment matched.* fields on the timeline', () => {
+  it.skip('Displays enrichment matched.* fields on the timeline', () => {


@ecezalp fyi. We need to get this in for 8.0, but just wanted you and the threat intel team to be aware

michaelolo24 · 2021-10-26T20:09:18Z

...plugins/security_solution/public/common/components/event_details/alert_summary_view.test.tsx

@@ -64,7 +64,7 @@ describe('AlertSummaryView', () => {
      expect(queryByTestId('summary-view-guide')).not.toBeInTheDocument();
    });
  });
-  test('Memory event code renders additional summary rows', () => {
+  test.skip('Memory event code renders additional summary rows', () => {


@angorayc - just an fyi since I think you worked on this?

michaelolo24 · 2021-10-26T20:11:52Z

...ins/security_solution/server/lib/detection_engine/routes/signals/query_signals_route.test.ts

-    context.core.elasticsearch.client.asCurrentUser.search.mockResolvedValue(
-      elasticsearchClientMock.createSuccessTransportRequestPromise(getEmptySignalsResponse())
-    );
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any


Just a minor nit if we can replace any here later on

michaelolo24 · 2021-10-26T20:16:42Z

x-pack/plugins/security_solution/common/constants.ts

-export const NOTIFICATION_THROTTLE_NO_ACTIONS = 'no_actions';
-export const NOTIFICATION_THROTTLE_RULE = 'rule';
+export const NOTIFICATION_THROTTLE_NO_ACTIONS = 'no_actions' as const;
+export const NOTIFICATION_THROTTLE_RULE = 'rule' as const;

 export const showAllOthersBucket: string[] = [
  'destination.ip',


TODO: Does 'signal.rule.threat.tactic.name' need to be replaced here as well?

michaelolo24

Thanks for doing this @madirey 💪🏾 🎉 ! Tested our critical paths and most of them look good, but I noted all my initial findings below. Approving so we can get it in and fix any bugs during our 8.0 cycle

I was able to switch the open, acknowledged, and close states successfully.
I was able to successfully open an alert in timeline, add to timeline, query in timeline, and open the analyzer when available
The table also successfully loads in the hosts page and the flyout there looks as expected as well

Just to note some things I noticed, and the people who (I believe) worked on them in case they get bug tickets (or just generally bugged) about the issues:

The details flyout loads data and shows what I would expect in the JSON and Table Fields, but there may be some fields missing in the overview and threat intel views. @angorayc and @ecezalp may be able to help confirm here
The context menu seems to have the case actions perpetually disabled, but that can probably be fixed in use_add_to_case.ts (fyi @kqualters-elastic)
The reason field renderer needs to be updated as it still references signal fields (fyi @machadoum)
Generally there are some additional signal-* references that probably need to be updated in the x-pack/plugins/timelines directory as well
@MadameSheema there will probably be some bugs that pop up as this work is the updated version of ([Security Solution][RAC] - Update UI signal references #107713) which we had discussed before, so the test plan we talked about for that work will apply here

stephmilovic · 2021-10-27T00:35:17Z

x-pack/plugins/security_solution/public/common/containers/source/index.tsx

@@ -254,7 +254,12 @@ export const useIndexFields = (sourcererScopeName: SourcererScopeName) => {
                      errorMessage: null,
                      id: sourcererScopeName,
                      indexPattern: getIndexFields(stringifyIndices, response.indexFields),
-                      indicesExist: response.indicesExist.length > 0,
+                      // If checking for DE signals index, lie and say the index is created (it's


dont do this! we will not have fields for signals index yet

I'm going to overwrite this tomorrow with #114806

stephmilovic · 2021-10-27T00:40:58Z

x-pack/plugins/security_solution/common/constants.ts

@@ -11,65 +11,65 @@ import type { TransformConfigSchema } from './transforms/types';
 import { ENABLE_CASE_CONNECTOR } from '../../cases/common';
 import { METADATA_TRANSFORMS_PATTERN } from './endpoint/constants';

-export const APP_ID = 'securitySolution';
+export const APP_ID = 'securitySolution' as const;


why? could you please explain this

do all new variables need this? i have a bunch of conflicts. a code comment would be super helpful if this is truly necessary, but I cannot understand why we would need this.

@stephmilovic The const assertions are not strictly necessary for the string literals, but have been used as a convention for consistency in other parts of the code base, e.g. constants in rule_registry (this also ensures that type widening does not occur if the value/type of the const is changed to a value which would be widened by TS by default). For other types (template strings, arrays, objects, etc.), as const ensures that the type cannot be widened, which provides a compile-time hint that is useful for IDEs. It was super useful in debugging the AAD changes to be able to have the IDE resolve the values of all the constants without having to go look them up every time. Happy to discuss!

…-migrate-away-from-injected-css-js * 'master' of github.com:elastic/kibana: (61 commits) [ML] Nodes overview for the Model Management page (elastic#116361) [Uptime] Uptime index config using kibana.yml (elastic#115775) [Controls] Dashboard Integration (elastic#115991) skip flaky suite (elastic#104260) Include Files in GitHub UI (elastic#115956) skip flaky suite (elastic#116060) [Canvas] By-Value Embeddables (elastic#113827) Skip failing test (elastic#115366) [Osquery] Fix live query search doesn't return relevant results for agents (elastic#116332) [Integrations] Added link in old Add Data description and fixed alignment in cards (elastic#116213) [Actions] Extended ActionTypeRegistry with connector validation to validate config with secrets (elastic#116079) skip flaky suite (elastic#109329) Grant access to machine learning features when base privileges are used (elastic#115444) Skipping failing test (elastic#84957) [RAC][Security Solution] Adds migration to new SecuritySolution rule types (elastic#112113) skip flaky suite (elastic#115366) [Fleet] Marking API spec as experimental (elastic#116331) [Docs] Cleaning up the versions in the upgrade paths. Closes elastic#116223 (elastic#116228) [Reporting] Suppress debug logs in the mock logger (elastic#116012) [Metrics UI] Clear threshold alert groups state when filterQuery changes (elastic#116205) ... # Conflicts: # src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx # src/plugins/dashboard/public/types.ts

jonathan-buttner · 2021-10-28T20:35:46Z

Hey @madirey as @michaelolo24 mentioned it looks like the menu actions for attaching alerts to cases are now disabled in this PR:

The context menu seems to have the case actions perpetually disabled, but that can probably be fixed in use_add_to_case.ts

I looked into it a bit and it seems like this component is getting an undefined event:
https://github.com/elastic/kibana/blob/master/x-pack/plugins/timelines/public/hooks/use_add_to_case.ts#L80

I just wanted to see if we already had a fix in the works, or if you knew what might be happening.

madirey · 2021-10-28T21:34:07Z

@jonathan-buttner That issue is documented, but we don't have a fix in the works yet. I'll be going through shortly to fix the skipped tests, which I think will uncover the issue (if nobody gets to it first). Most likely something is still looking for a legacy field name, and so the event is not resolved properly. Perhaps here? https://github.com/elastic/kibana/blob/master/x-pack/plugins/timelines/public/hooks/use_add_to_case.ts#L129

madirey · 2021-10-29T13:55:09Z

@jonathan-buttner That issue is documented, but we don't have a fix in the works yet. I'll be going through shortly to fix the skipped tests, which I think will uncover the issue (if nobody gets to it first). Most likely something is still looking for a legacy field name, and so the event is not resolved properly. Perhaps here? https://github.com/elastic/kibana/blob/master/x-pack/plugins/timelines/public/hooks/use_add_to_case.ts#L129

@michaelolo24 @XavierM

PR: #116768

@PhilippeOberti

…74216) ## Summary We just got an [SDH#814](elastic/sdh-security-team#814) that tell us that some feature like `KPIs` and `grouping` are not acting as they should be. @PhilippeOberti is doing an investigation to check which feature has been impacted by this bug. This bug has been introduced in this #112113 since 8.0.0 I think this simple solution should not impact any features.

@PhilippeOberti

…astic#174216) ## Summary We just got an [SDH#814](elastic/sdh-security-team#814) that tell us that some feature like `KPIs` and `grouping` are not acting as they should be. @PhilippeOberti is doing an investigation to check which feature has been impacted by this bug. This bug has been introduced in this elastic#112113 since 8.0.0 I think this simple solution should not impact any features. (cherry picked from commit 4af36fe)

@PhilippeOberti

…74216) We just got an [SDH#814](elastic/sdh-security-team#814) that tell us that some feature like `KPIs` and `grouping` are not acting as they should be. @PhilippeOberti is doing an investigation to check which feature has been impacted by this bug. This bug has been introduced in this #112113 since 8.0.0 I think this simple solution should not impact any features. (cherry picked from commit 4af36fe)

…ser (#174216) (#174304) # Backport This will backport the following commits from `main` to `8.12`: - [[SECURITY_SOLUTIONS] Only query security alerts with current user (#174216)](#174216)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>

…ser (#174216) (#174306) # Backport This will backport the following commits from `main` to `8.11`: - [[SECURITY_SOLUTIONS] Only query security alerts with current user (#174216)](#174216)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

madirey added 24 commits September 7, 2021 13:05

Initial commit

82e35be

Properly handle signal history

3ae81a7

Merge branch 'master' of github.com:elastic/kibana into security-rule…

bb739d4

…-type-threshold

Fix elastic#95258 - cardinality sort bug

ee7ce2a

Merge branch 'master' of github.com:elastic/kibana into security-rule…

5ede622

…-type-threshold

Init threshold rule

e28c671

Merge branch 'master' of github.com:elastic/kibana into security-rule…

5d1f81d

…-type-threshold

Create working threshold rule

958640f

Fix threshold signal generation

b29aee9

Fix tests

851301b

Merge branch 'master' of github.com:elastic/kibana into security-rule…

123d6f3

…-type-threshold

Update mappings

847876f

Merge branch 'master' of github.com:elastic/kibana into security-rule…

0859f5a

…-type-threshold

ALERT_TYPE_ID => RULE_TYPE_ID

0e7676d

Merge branch 'master' of github.com:elastic/kibana into security-rule…

6057c94

…-type-threshold

Merge branch 'master' of github.com:elastic/kibana into security-rule…

d3d14fa

…-type-threshold

Add tests

2082b3b

Fix types

a352cbb

Merge branch 'master' of github.com:elastic/kibana into security-rule…

04a024d

…-type-threshold

Adds RAC rule type migration

8b81415

Merge branch 'master' of github.com:elastic/kibana into security-rule…

62c3dcb

…-type-threshold

Fix threshold tests (remove outputIndex)

f250676

Add threshold rule type to ruleTypeMappings

bc969c2

Merge branch 'security-rule-type-threshold' into security-rac-rules-m…

e257b30

…igration

madirey added 3 commits October 26, 2021 11:08

Merge branch 'master' of github.com:elastic/kibana into security-rac-…

27fc6e6

…rules-migration

Merge branch 'master' of github.com:elastic/kibana into security-rac-…

8ef98a7

…rules-migration

Merge branch 'master' of github.com:elastic/kibana into security-rac-…

5f70fc6

…rules-migration

michaelolo24 reviewed Oct 26, 2021

View reviewed changes

michaelolo24 approved these changes Oct 26, 2021

View reviewed changes

madirey merged commit 117efdf into elastic:master Oct 26, 2021

Kibana Alerting automation moved this from In Review to Done (Ordered by most recent) Oct 26, 2021

stephmilovic reviewed Oct 27, 2021

View reviewed changes

michaelolo24 mentioned this pull request Oct 27, 2021

[Security Solution][RAC] - Update UI signal references #107713

Closed

MadameSheema mentioned this pull request Oct 27, 2021

[Security Solution] Skips EQL test #116456

Merged

madirey mentioned this pull request Oct 27, 2021

Unskip tests for RAC rule migration #116460

Closed

20 tasks

madirey added the backport:skip This commit does not require backporting label Oct 27, 2021

MadameSheema mentioned this pull request Oct 28, 2021

[Security Solution] Skips failing tests #116568

Merged

madirey deleted the security-rac-rules-migration branch October 28, 2021 21:30

madirey mentioned this pull request Nov 9, 2021

[Security Solution][RAC] Fixes Threshold Rule functionality for post-rule-migration 8.0 #117571

Merged

9 tasks

XavierM mentioned this pull request Jan 3, 2024

[SECURITY_SOLUTIONS] Only query security alerts with current user #174216

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC][Security Solution] Adds migration to new SecuritySolution rule types #112113

[RAC][Security Solution] Adds migration to new SecuritySolution rule types #112113

madirey commented Sep 14, 2021 •

edited

kibanamachine commented Oct 26, 2021

API count

michaelolo24 Oct 26, 2021

michaelolo24 Oct 26, 2021

michaelolo24 Oct 26, 2021

michaelolo24 Oct 26, 2021

michaelolo24 left a comment

stephmilovic Oct 27, 2021 •

edited

stephmilovic Oct 27, 2021

stephmilovic Oct 27, 2021 •

edited

stephmilovic Oct 27, 2021

madirey Oct 27, 2021 •

edited

jonathan-buttner commented Oct 28, 2021

madirey commented Oct 28, 2021

madirey commented Oct 29, 2021 •

edited

[RAC][Security Solution] Adds migration to new SecuritySolution rule types #112113

[RAC][Security Solution] Adds migration to new SecuritySolution rule types #112113

Conversation

madirey commented Sep 14, 2021 • edited

Summary

Changes include

To be addressed later

Checklist

Risk Matrix

For maintainers

kibanamachine commented Oct 26, 2021

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

michaelolo24 Oct 26, 2021

Choose a reason for hiding this comment

michaelolo24 Oct 26, 2021

Choose a reason for hiding this comment

michaelolo24 Oct 26, 2021

Choose a reason for hiding this comment

michaelolo24 Oct 26, 2021

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

stephmilovic Oct 27, 2021 • edited

Choose a reason for hiding this comment

stephmilovic Oct 27, 2021

Choose a reason for hiding this comment

stephmilovic Oct 27, 2021 • edited

Choose a reason for hiding this comment

stephmilovic Oct 27, 2021

Choose a reason for hiding this comment

madirey Oct 27, 2021 • edited

Choose a reason for hiding this comment

jonathan-buttner commented Oct 28, 2021

madirey commented Oct 28, 2021

madirey commented Oct 29, 2021 • edited

madirey commented Sep 14, 2021 •

edited

stephmilovic Oct 27, 2021 •

edited

stephmilovic Oct 27, 2021 •

edited

madirey Oct 27, 2021 •

edited

madirey commented Oct 29, 2021 •

edited