[Security Solution] Add Threat Match rule specific editable fields #200308

maximpn · 2024-11-15T15:40:20Z

Partially addresses: #171520

Summary

This PR adds is built on top of #193828 and #196948 and adds the following editable components for Threat Match rule type

threat_index
threat_query
threat_mapping
threat_indicator_path
~~threat_language~~ threat_language was merged with threat_query

Details

This PR make a set of changes to make existing Threat Match form fields easily reusable as editable components and type safe when used in forms. In particular the following was done

Fixes a bug blocking Threat Match rules upgrading
Existing functionality was refactored to have reusable self-contained editable components for threat_index, threat_query, threat_mapping and threat_indicator_path rule fields
threat_language was removed since query type is included in threat_query field and can be edited with Query Bar
threat mapping input was split into separate component for individual fields to be reused
ThreatMatchComponent was refactored to be a controlled component instead of uncontrolled
ThreatMatchComponent has a feature preventing users removing the single last entry. Instead deleting the last entry the delete button clears inputs. That functionality didn't work properly in Prebuilt Rule Customization workflow and rule creation/editing forms after creating a reusable ThreatMappingEdit component. Instead of trying to find a tricky fix ThreatMatchComponent was refactored to remove internal state. The feature preventing users removing the single last entry was reimplemented in ThreatMappingEdit component.
Fixes a bug reproducible in main where validation errors duplicated described in a comment
Fixes a bug reproducible in main allowing to save unknown source indices or indicator indices fields described in a comment

How to test

Ensure the prebuiltRulesCustomizationEnabled feature flag is enabled
Allow internal APIs via adding server.restrictInternalApis: false to kibana.dev.yaml
Clear Elasticsearch data
Run Elasticsearch and Kibana locally (do not open Kibana in a web browser)
Install an outdated version of the security_detection_engine Fleet package

curl -X POST --user elastic:changeme  -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H "elastic-api-version: 2023-10-31" -d '{"force":true}' http://localhost:5601/kbn/api/fleet/epm/packages/security_detection_engine/8.14.1

Install prebuilt rules

curl -X POST --user elastic:changeme  -H 'Content-Type: application/json' -H 'kbn-xsrf: 123' -H "elastic-api-version: 1" -d '{"mode":"ALL_RULES"}' http://localhost:5601/kbn/internal/detection_engine/prebuilt_rules/installation/_perform

Open a threat_match rule for editing. For example Threat Intel Hash Indicator Match with rule_id aab184d3-72b3-4639-b242-6597c99d8bca.
Edit Indicator index patterns, Indicator index query and/or Indicator filters, Indicator mapping and Indicator prefix override fields
Open Detection Rules (SIEM) Page -> Rule Updates -> click on Threat Intel Hash Indicator Match rule -> expand each Threat Match rule type specific field -> press Edit button

Screenshots

Threat Match Query edit component

Threat Match Index edit component

Threat Match Mapping edit component

Threat Match Indicator Path edit component

Threat Match Mapping unknown field names validation warnings

elasticmachine · 2024-11-17T22:03:27Z

Pinging @elastic/security-detections-response (Team:Detections and Resp)

elasticmachine · 2024-11-17T22:03:28Z

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine · 2024-11-17T22:03:29Z

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

vitaliidm

I have found few issues and left some comments, mostly questions and suggestions.
Will continue testing

...tection_engine/rule_creation/components/threat_match_index_edit/threat_match_index_field.tsx

...rity_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx

...ion_engine/rule_creation_ui/components/step_define_rule/use_persistent_threat_match_state.ts

...etection_engine/rule_creation/components/threat_match_index_edit/threat_match_index_edit.tsx

...rity_solution/public/detection_engine/rule_creation_ui/components/description_step/index.tsx

...olution/common/api/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts

x-pack/plugins/security_solution/public/common/components/threat_match/index.tsx

...olution/common/api/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts

xcrzx · 2024-11-20T13:41:37Z

I don’t have any changes in the filters field, but the diff shows some empty meta.alias fields that it thinks should be removed:

What is the meta field, and should it even be part of the diff calculation?

cc @dplumlee This seems similar to the issue we recently had with the schedule and threat fields.

xcrzx · 2024-11-20T14:06:45Z

The incorrect final field version has been selected. I have modifications to the field, so in this case, we should pre-select the current version and preserve all user modifications instead of removing them.

maximpn · 2024-12-19T10:47:03Z

...ion_engine/rule_creation/components/threat_match_mapping_edit/threat_match_mapping_field.tsx

+  // We have to make sure validation runs against the latest source events index patterns
+  // and threat match index patterns.
+  // Form lib's `fieldsToValidateOnChange` on the corresponding index patterns edit fields
+  // doesn't help here. It leads to running threat match mapping validation before render
+  // of this component happens. In the end validation runs against previous index patterns
+  // producing invalid validation results.
+  //
+  // Validating the field imperatively here fixes this issue.
+  //
+  // Additional pitfall here is `validate` function changing its reference every render. Including it
+  // in useEffect's deps leads to infinite re-render.
+  useEffect(() => {
+    validate();
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [indexPatterns, threatIndexPatterns]);


This part looks sub-optimal but it's the best I was able to make to re-run threat match mapping validation upon index pattern changes. An alternative is using formData in validation to read index patterns/data source, transform them to data view asynchronously and then validate fields against known fields. Alternative's drawback is difficulty to reuse it in rule upgrade workflow.

Any bright ideas are welcome.

vitaliidm · 2024-12-19T14:44:41Z

...ion_engine/rule_creation_ui/components/step_define_rule/use_persistent_threat_match_state.ts

+import { useFormData } from '../../../../shared_imports';
+
+interface UsePersistentThreatMatchStateParams {
+  form: FormHook;


some of the comments still not addressed: #200308 (comment)

vitaliidm · 2024-12-19T14:45:04Z

...ion_engine/rule_creation_ui/components/step_define_rule/use_persistent_threat_match_state.ts

+export function usePersistentThreatMatchState({ form }: UsePersistentThreatMatchStateParams): void {
+  const lastThreatMatchState = useRef<LastThreatMatchState | undefined>();
+  const [{ ruleType, threatIndex: threatIndexPatterns, threatQueryBar, threatMapping }] =
+    useFormData({ form, watch: ['ruleType', 'threatIndex', 'threatQueryBar', 'threatMapping'] });


some of the comments still not addressed: #200308 (comment)

vitaliidm

There are few minor comments and mostly discussion over #200308 (comment)
I would like to be addressed before merge

About
#200308 (comment)
I think the proper solution should involve async validation.
At the same time, in alternative approach, are multiple requests to get index fields sent? (from created data view in validator and data views from parent component)
They might be very heavy, so ideally if we want to do them once.

vitaliidm · 2024-12-19T17:48:29Z

...on/components/threat_match_mapping_edit/validators/threat_match_mapping_validator_factory.ts

+          'xpack.securitySolution.detectionEngine.ruleManagement.threatMappingField.unknownFields',
+          {
+            defaultMessage:
+              'Indicator mapping has unknown fields. {unknownSourceIndicesFields} field(s) not found in the source events indices and {unknownThreatMatchIndicesFields} field(s) not found in the indicator indices.',


we should use plurals instead of field(s)

vitaliidm · 2024-12-19T17:48:40Z

...on/components/threat_match_mapping_edit/validators/threat_match_mapping_validator_factory.ts

+          'xpack.securitySolution.detectionEngine.ruleManagement.threatMappingField.unknownSourceIndicesFields',
+          {
+            defaultMessage:
+              'Indicator mapping has unknown fields. {unknownSourceIndicesFields} field(s) not found in the source events indices.',


we should use plurals instead of field(s)

vitaliidm · 2024-12-19T17:48:50Z

...on/components/threat_match_mapping_edit/validators/threat_match_mapping_validator_factory.ts

+          'xpack.securitySolution.detectionEngine.ruleManagement.threatMappingField.unknownIndicatorIndicesFields',
+          {
+            defaultMessage:
+              'Indicator mapping has unknown fields. {unknownThreatMatchIndicesFields} field(s) not found in the indicator indices.',


we should use plurals instead of field(s)

elasticmachine · 2024-12-19T20:37:43Z

💔 Build Failed

Buildkite Build
Commit: 3cd7d30

Failed CI Steps

Check Types

History

💚 Build #262212 succeeded 5ed85f5
💛 Build #262077 was flaky 1ab0f3c
💔 Build #261886 failed 6d84844
💔 Build #261868 failed 243d1e7
💔 Build #261786 failed b7db6d8

cc @maximpn

maximpn self-assigned this Nov 15, 2024

maximpn force-pushed the add-threat-match-specific-editable-fields branch 2 times, most recently from 018a0c9 to 9b576a1 Compare November 16, 2024 11:02

maximpn requested a review from nikitaindik November 17, 2024 22:03

maximpn marked this pull request as ready for review November 17, 2024 22:03

maximpn requested review from a team as code owners November 17, 2024 22:03

maximpn requested a review from vitaliidm November 17, 2024 22:03

maximpn mentioned this pull request Nov 18, 2024

[Security Solution] Rework Update flyout to display all field updates and build Three-Way-Diff field component #171520

Open

69 tasks

xcrzx self-requested a review November 18, 2024 09:52

vitaliidm reviewed Nov 19, 2024

View reviewed changes

nikitaindik removed their request for review November 21, 2024 10:51

maximpn added v8.18.0 and removed v8.17.0 labels Nov 22, 2024

maximpn force-pushed the add-threat-match-specific-editable-fields branch from e0ff1dd to f9e8e69 Compare November 27, 2024 21:50

maximpn requested a review from vitaliidm November 27, 2024 21:52

maximpn added 23 commits December 19, 2024 08:20

remove unused translation keys

f527873

use EUI theme for custom styling

3feb116

use hardcoded form paths

6562e5a

move threat index patterns validator to a separate folder

f46d520

add threat index patterns description step tests

92a3a31

include threat match fields for rule preview availability check

e420b96

rename a component for better readability

cda292b

show threat language along threat query

d47dd70

fix validation upon saving

9fc4d97

show field values non found in data views

5708cf3

fix threat match prebuilt rules upgrade functionality

52584e8

fix a broken unit test

d339928

use hardcoded ruleType form field name

0a8d29b

fixed after rebasing

37144ed

after rebase fix

cfacee3

simplify andLogicIncluded condition

7797692

fix threat match query validation error message

bd4e651

fix rule preview disabled state on rule editing page

01b913d

fix duplicating validation error messages

69e7207

post-rebase conflicts resolving

b9ffde1

add warning validation for indicator mapping field names

20905ab

fix after-rebase issues

f15a0dd

force threat match mapping validation upon index patterns change

5ed85f5

maximpn force-pushed the add-threat-match-specific-editable-fields branch from 1ab0f3c to 5ed85f5 Compare December 19, 2024 10:34

maximpn commented Dec 19, 2024

View reviewed changes

vitaliidm reviewed Dec 19, 2024

View reviewed changes

maximpn added 2 commits December 19, 2024 20:11

specify field types in usePersistentThreatMatchState

9ccebd6

stick to plurals in validation error messages

3cd7d30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Add Threat Match rule specific editable fields #200308

[Security Solution] Add Threat Match rule specific editable fields #200308

maximpn commented Nov 15, 2024 •

edited

Loading

elasticmachine commented Nov 17, 2024

elasticmachine commented Nov 17, 2024

elasticmachine commented Nov 17, 2024

vitaliidm left a comment

xcrzx commented Nov 20, 2024

xcrzx commented Nov 20, 2024

maximpn Dec 19, 2024

vitaliidm Dec 19, 2024

vitaliidm Dec 19, 2024

vitaliidm left a comment

vitaliidm Dec 19, 2024

vitaliidm Dec 19, 2024

vitaliidm Dec 19, 2024

elasticmachine commented Dec 19, 2024 •

edited

Loading

[Security Solution] Add Threat Match rule specific editable fields #200308

Are you sure you want to change the base?

[Security Solution] Add Threat Match rule specific editable fields #200308

Conversation

maximpn commented Nov 15, 2024 • edited Loading

Summary

Details

How to test

Screenshots

elasticmachine commented Nov 17, 2024

elasticmachine commented Nov 17, 2024

elasticmachine commented Nov 17, 2024

vitaliidm left a comment

Choose a reason for hiding this comment

xcrzx commented Nov 20, 2024

xcrzx commented Nov 20, 2024

maximpn Dec 19, 2024

Choose a reason for hiding this comment

vitaliidm Dec 19, 2024

Choose a reason for hiding this comment

vitaliidm Dec 19, 2024

Choose a reason for hiding this comment

vitaliidm left a comment

Choose a reason for hiding this comment

vitaliidm Dec 19, 2024

Choose a reason for hiding this comment

vitaliidm Dec 19, 2024

Choose a reason for hiding this comment

vitaliidm Dec 19, 2024

Choose a reason for hiding this comment

elasticmachine commented Dec 19, 2024 • edited Loading

💔 Build Failed

Failed CI Steps

History

maximpn commented Nov 15, 2024 •

edited

Loading

elasticmachine commented Dec 19, 2024 •

edited

Loading