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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Rules mistakenly marked as customized #199629

Closed
Tracked by #201502
xcrzx opened this issue Nov 11, 2024 · 3 comments · Fixed by #200105
Closed
Tracked by #201502

[Security Solution] Rules mistakenly marked as customized #199629

xcrzx opened this issue Nov 11, 2024 · 3 comments · Fixed by #200105
Assignees
@dplumlee
Labels
8.17 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.1 v8.17.0 v9.0.0

Comments

@xcrzx
Copy link
Contributor

xcrzx commented Nov 11, 2024

Summary

A prebuilt rule is marked as customized when an action is attached to it. The diff algorithm incorrectly detects changes to the threat and rule_schedule fields.

Steps to Reproduce

  1. Open any prebuilt rule for editing.
  2. With the feature flag off, attaching actions is the only possible edit to prebuilt rules. So, add a rule action and save the rule.
  3. Read the rule using the rule read API.

Expected Result

Since adding or changing rule actions should not mark rules as customized, the returned rule should have rule_source.is_customized = false.

Actual Result

The rule is incorrectly marked as customized.

Initial Analysis

The rule asset object and installed rule saved object have some values represented in different formats, leading to mismatches. The differences include:

  • rule_schedule is in minutes (e.g., 4m) in one case and seconds (e.g., 240s) in another.
  • threat may omit optional fields, such as subtechnique, in the rule asset object, whereas the rule saved object has them as empty arrays (e.g., subtechnique: []).

Other fields may also have similar format differences. To address this, the diff algorithm should:

  • Account for values like empty arrays being equivalent to missing values for certain fields.
  • Normalize units before comparison (e.g., convert minutes to seconds).

This ensures consistency in identifying rule changes.
@xcrzx xcrzx added bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team v8.16.1 v8.17.0 v9.0.0 labels Nov 11, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@xcrzx xcrzx mentioned this issue Nov 11, 2024
Open
@banderror banderror added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. 8.17 candidate labels Nov 11, 2024
@dplumlee dplumlee mentioned this issue Nov 13, 2024
Merged
2 tasks
@banderror banderror mentioned this issue Nov 13, 2024
Merged
3 tasks
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 18, 2024
@dplumlee
[Security Solution] Fixes data normalization in diff algorithms for `…
7e214ac 
…threat` and `rule_schedule` fields (elastic#200105)

**Fixes elastic#199629

## Summary

Fixes the data normalization we do before comparison for the `threat`
and `rule_schedule` fields so that they align with our prebuilt rule
specs. Specifically:

- Trims any extra optional nested fields in the `threat` field that were
left as empty arrays
- Removes the logic to use the `from` value in the `meta` field if it
existed, so that we can normalize the time strings for `rule_schedule`

These errors were occurring when a rule was saved via the Rule Editing
form in the UI and extra fields were added in the update API call. This
PR makes the diff algorithms more robust against different field values
that are represented differently but are logically the same.

This extra data added in the Rule Edit UI form was also causing rules to
appear as modified when saved from the form, even if no fields had been
modified.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [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

(cherry picked from commit a8fd0c9)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Nov 18, 2024
@dplumlee
[Security Solution] Fixes data normalization in diff algorithms for `…
f463c40 
…threat` and `rule_schedule` fields (elastic#200105)

**Fixes elastic#199629

## Summary

Fixes the data normalization we do before comparison for the `threat`
and `rule_schedule` fields so that they align with our prebuilt rule
specs. Specifically:

- Trims any extra optional nested fields in the `threat` field that were
left as empty arrays
- Removes the logic to use the `from` value in the `meta` field if it
existed, so that we can normalize the time strings for `rule_schedule`

These errors were occurring when a rule was saved via the Rule Editing
form in the UI and extra fields were added in the update API call. This
PR makes the diff algorithms more robust against different field values
that are represented differently but are logically the same.

This extra data added in the Rule Edit UI form was also causing rules to
appear as modified when saved from the form, even if no fields had been
modified.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [x] [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

(cherry picked from commit a8fd0c9)
@banderror banderror mentioned this issue Nov 23, 2024
Open
@dplumlee dplumlee mentioned this issue Nov 26, 2024
Closed
@banderror banderror mentioned this issue Nov 27, 2024
Open
5 tasks
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this issue Dec 12, 2024
@dplumlee @CAWilson94
[Security Solution] Fixes data normalization in diff algorithms for `…
734f661 
…threat` and `rule_schedule` fields (elastic#200105)

**Fixes elastic#199629

## Summary

Fixes the data normalization we do before comparison for the `threat`
and `rule_schedule` fields so that they align with our prebuilt rule
specs. Specifically:

- Trims any extra optional nested fields in the `threat` field that were
left as empty arrays
- Removes the logic to use the `from` value in the `meta` field if it
existed, so that we can normalize the time strings for `rule_schedule`

These errors were occurring when a rule was saved via the Rule Editing
form in the UI and extra fields were added in the update API call. This
PR makes the diff algorithms more robust against different field values
that are represented differently but are logically the same.

This extra data added in the Rule Edit UI form was also causing rules to
appear as modified when saved from the form, even if no fields had been
modified.



### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [x] [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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dplumlee dplumlee

Labels
8.17 candidate bug Fixes for quality problems that affect the customer experience Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.1 v8.17.0 v9.0.0
Projects
None yet
Milestone
No milestone
4 participants
@xcrzx @banderror @elasticmachine @dplumlee