From 9e52f7064f4016b9c52606ab865149d6b48f5ae4 Mon Sep 17 00:00:00 2001 From: Georgii Gorbachev Date: Wed, 5 Jul 2023 22:42:21 +0200 Subject: [PATCH] [Security Solution] Support rule type changes in the rule upgrade workflow (#161247) **Fixes: /~https://github.com/elastic/kibana/issues/161094** ## Summary - Adds support for rule type changes in the `/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint. - Previously, if any rule had a different `type` in its `current_version` compared to its `target_version` the request would fail with `500`. - This PR: - updates this behaviour to accept rule type changes - creates a new `calculateAllFieldsDiff` method that is responsible for calculating diffs among all fields of all rule types. Used exclusively when there has been a rule type change between the current version and the target version (which can normally happen through upgrades of the `security_detection_engine` package) OR when the base version has a different type as the current version (which should not happen under normal conditions and user behaviour). - updates the diffable fields types for each specifc rule type (e.g.: `DiffableCustomQueryFields`,`DiffableEqlFields`,`DiffableThreatMatchFields`, etc) , replacing the `data_query` field name for either `eql_query` (for EQL type rules) or `kql_query` (for all others). ## How to test 1. With a clean Kibana state, use the `xpack.securitySolution.prebuiltRulesPackageVersion` config to force Kibana to install a package that contains the rules with their original type: ``` xpack.securitySolution.prebuiltRulesPackageVersion: '8.3.1' ``` 2. Install the four "offending" rules, [listed below.](/~https://github.com/elastic/kibana/pull/161247#issuecomment-1622132120) 3. Remove the config, restart Kibana and navigate to the Rules Page so that the latest package is installed. 4. Navigate to the Rule Updates table. The four installed rules should have updates available. Update them. 5. All the listed rule types should be updated, as well as their corresponding fields. ### 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: jpdjere --- .../model/diff/diffable_rule/diffable_rule.ts | 30 ++++++++--- .../model/diff/rule_diff/rule_diff.ts | 2 + .../calculation/calculate_rule_fields_diff.ts | 50 ++++++++++++++++--- .../normalization/convert_rule_to_diffable.ts | 12 ++--- 4 files changed, 76 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts b/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts index 81dc7741a2bf4..8ce2fb9bf7f1a 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule.ts @@ -114,7 +114,7 @@ export type DiffableCustomQueryFields = t.TypeOf export const DiffableSavedQueryFields = buildSchema({ required: { type: t.literal('saved_query'), - data_query: RuleKqlQuery, // NOTE: new field + kql_query: RuleKqlQuery, // NOTE: new field }, optional: { data_source: RuleDataSource, // NOTE: new field @@ -138,7 +138,7 @@ export type DiffableEqlFields = t.TypeOf; export const DiffableEqlFields = buildSchema({ required: { type: t.literal('eql'), - data_query: RuleEqlQuery, // NOTE: new field + eql_query: RuleEqlQuery, // NOTE: new field }, optional: { data_source: RuleDataSource, // NOTE: new field @@ -152,7 +152,7 @@ export type DiffableThreatMatchFields = t.TypeOf; export const DiffableThresholdFields = buildSchema({ required: { type: t.literal('threshold'), - data_query: RuleKqlQuery, // NOTE: new field + kql_query: RuleKqlQuery, // NOTE: new field threshold: Threshold, }, optional: { @@ -191,7 +191,7 @@ export type DiffableNewTermsFields = t.TypeOf; export const DiffableNewTermsFields = buildSchema({ required: { type: t.literal('new_terms'), - data_query: InlineKqlQuery, // NOTE: new field + kql_query: InlineKqlQuery, // NOTE: new field new_terms_fields: NewTermsFields, history_window_start: HistoryWindowStart, }, @@ -239,3 +239,21 @@ export const DiffableRule = t.intersection([ DiffableNewTermsFields, ]), ]); + +/** + * This is a merge of all fields from all rule types into a single TS type. + * This is NOT a union discriminated by rule type, as DiffableRule is. + */ +export type DiffableAllFields = DiffableCommonFields & + Omit & + Omit & + Omit & + Omit & + Omit & + Omit & + Omit & + DiffableRuleTypeField; + +interface DiffableRuleTypeField { + type: DiffableRule['type']; +} diff --git a/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff.ts b/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff.ts index 8a28b358f5303..bdc9b1e2a6e2c 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/prebuilt_rules/model/diff/rule_diff/rule_diff.ts @@ -6,6 +6,7 @@ */ import type { + DiffableAllFields, DiffableCommonFields, DiffableCustomQueryFields, DiffableEqlFields, @@ -18,6 +19,7 @@ import type { import type { FieldsDiff } from './fields_diff'; +export type AllFieldsDiff = FieldsDiff; export type CommonFieldsDiff = FieldsDiff; export type CustomQueryFieldsDiff = FieldsDiff; export type SavedQueryFieldsDiff = FieldsDiff; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts index abd084c1789e4..4525556222331 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts @@ -9,6 +9,7 @@ import { assertUnreachable } from '../../../../../../../common/utility_types'; import { invariant } from '../../../../../../../common/utils/invariant'; import type { + DiffableAllFields, DiffableCommonFields, DiffableCustomQueryFields, DiffableEqlFields, @@ -20,6 +21,7 @@ import type { DiffableThresholdFields, } from '../../../../../../../common/detection_engine/prebuilt_rules/model/diff/diffable_rule/diffable_rule'; import type { + AllFieldsDiff, CommonFieldsDiff, CustomQueryFieldsDiff, EqlFieldsDiff, @@ -53,6 +55,24 @@ export const calculateRuleFieldsDiff = ( const { base_version, current_version, target_version } = ruleVersions; const hasBaseVersion = base_version !== MissingVersion; + const isRuleTypeDifferentInTargetVersion = current_version.type !== target_version.type; + const isRuleTypeDifferentInBaseVersion = hasBaseVersion + ? current_version.type !== base_version.type + : false; + + if (isRuleTypeDifferentInTargetVersion || isRuleTypeDifferentInBaseVersion) { + // If rule type has been changed by Elastic in the target version (can happen) + // or by user in the current version (should never happen), we can't calculate the diff + // only for fields of a single rule type, and need to calculate it for all fields + // of all the rule types we have. + // TODO: Try to get rid of "as" casting + return calculateAllFieldsDiff({ + base_version: base_version as DiffableAllFields | MissingVersion, + current_version: current_version as DiffableAllFields, + target_version: target_version as DiffableAllFields, + }) as RuleFieldsDiff; + } + switch (current_version.type) { case 'query': { if (hasBaseVersion) { @@ -175,7 +195,7 @@ const calculateCustomQueryFieldsDiff = ( const customQueryFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + kql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, alert_suppression: simpleDiffAlgorithm, }; @@ -188,7 +208,7 @@ const calculateSavedQueryFieldsDiff = ( const savedQueryFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + kql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, alert_suppression: simpleDiffAlgorithm, }; @@ -201,7 +221,7 @@ const calculateEqlFieldsDiff = ( const eqlFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + eql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, event_category_override: simpleDiffAlgorithm, timestamp_field: simpleDiffAlgorithm, @@ -216,7 +236,7 @@ const calculateThreatMatchFieldsDiff = ( const threatMatchFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + kql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, threat_query: simpleDiffAlgorithm, threat_index: simpleDiffAlgorithm, @@ -234,7 +254,7 @@ const calculateThresholdFieldsDiff = ( const thresholdFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + kql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, threshold: simpleDiffAlgorithm, }; @@ -260,8 +280,26 @@ const calculateNewTermsFieldsDiff = ( const newTermsFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { type: simpleDiffAlgorithm, - data_query: simpleDiffAlgorithm, + kql_query: simpleDiffAlgorithm, data_source: simpleDiffAlgorithm, new_terms_fields: simpleDiffAlgorithm, history_window_start: simpleDiffAlgorithm, }; + +const calculateAllFieldsDiff = ( + ruleVersions: ThreeVersionsOf +): AllFieldsDiff => { + return calculateFieldsDiffFor(ruleVersions, allFieldsDiffAlgorithms); +}; + +const allFieldsDiffAlgorithms: FieldsDiffAlgorithmsFor = { + ...commonFieldsDiffAlgorithms, + ...customQueryFieldsDiffAlgorithms, + ...savedQueryFieldsDiffAlgorithms, + ...eqlFieldsDiffAlgorithms, + ...threatMatchFieldsDiffAlgorithms, + ...thresholdFieldsDiffAlgorithms, + ...machineLearningFieldsDiffAlgorithms, + ...newTermsFieldsDiffAlgorithms, + type: simpleDiffAlgorithm, +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/normalization/convert_rule_to_diffable.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/normalization/convert_rule_to_diffable.ts index f917b7154a163..b1838d77dfeb8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/normalization/convert_rule_to_diffable.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/normalization/convert_rule_to_diffable.ts @@ -146,7 +146,7 @@ const extractDiffableCustomQueryFields = ( ): DiffableCustomQueryFields => { return { type: rule.type, - data_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), + kql_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), data_source: extractRuleDataSource(rule.index, rule.data_view_id), alert_suppression: rule.alert_suppression, }; @@ -157,7 +157,7 @@ const extractDiffableSavedQueryFieldsFromRuleObject = ( ): DiffableSavedQueryFields => { return { type: rule.type, - data_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), + kql_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), data_source: extractRuleDataSource(rule.index, rule.data_view_id), alert_suppression: rule.alert_suppression, }; @@ -168,7 +168,7 @@ const extractDiffableEqlFieldsFromRuleObject = ( ): DiffableEqlFields => { return { type: rule.type, - data_query: extractRuleEqlQuery(rule.query, rule.language, rule.filters), + eql_query: extractRuleEqlQuery(rule.query, rule.language, rule.filters), data_source: extractRuleDataSource(rule.index, rule.data_view_id), event_category_override: rule.event_category_override, timestamp_field: rule.timestamp_field, @@ -181,7 +181,7 @@ const extractDiffableThreatMatchFieldsFromRuleObject = ( ): DiffableThreatMatchFields => { return { type: rule.type, - data_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), + kql_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), data_source: extractRuleDataSource(rule.index, rule.data_view_id), threat_query: extractInlineKqlQuery( rule.threat_query, @@ -201,7 +201,7 @@ const extractDiffableThresholdFieldsFromRuleObject = ( ): DiffableThresholdFields => { return { type: rule.type, - data_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), + kql_query: extractRuleKqlQuery(rule.query, rule.language, rule.filters, rule.saved_id), data_source: extractRuleDataSource(rule.index, rule.data_view_id), threshold: rule.threshold, }; @@ -222,7 +222,7 @@ const extractDiffableNewTermsFieldsFromRuleObject = ( ): DiffableNewTermsFields => { return { type: rule.type, - data_query: extractInlineKqlQuery(rule.query, rule.language, rule.filters), + kql_query: extractInlineKqlQuery(rule.query, rule.language, rule.filters), data_source: extractRuleDataSource(rule.index, rule.data_view_id), new_terms_fields: rule.new_terms_fields, history_window_start: rule.history_window_start,