-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Rule Updates table has broken and users can't upgrade prebuilt rules #161094
Comments
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Thanks for bringing this to our attention. We removed the rule type change restriction after having a discussion with your team. We can turn this restriction back on if necessary after we fix the packages. Reference: elastic/detection-rules#2769 Do we know which specific rules caused this to error? I'll take a moment to find the offending rules shortly but wanted to ask first. The |
@terrancedejesus Thank you for your quick response!
I guess this comment was correct on May 1st but this changed in 2 months during which we finalized the feature and made it ready for release in Yes, please turn the restriction back -- we won't support rule type updates in the new API endpoints and UI for prebuilt rules.
@jpdjere can debug this locally and find all the rules that are causing the bug.
Okay, yeah, I think we should fix it for those packages as well by releasing new patch versions for them. Though it's not that critical compared to fixing the |
@banderror thanks for the follow up! No problem about the new restrictions, glad we can fix them before release. These are the only rule type changes I have found since the Query Type rules:
@jpdjere - From what @banderror said, the
Questions:
Example of rule type change from tuning from kuery (query) to kuery (new_terms): I deployed an UpdateInstalled an agent on Linux pointing to this stack and then simulated some activity to trigger an alert for one of these rules with rule type changes. Alert generated properly. |
@terrancedejesus The issue shows up only when the user has any of the rules that had their type updated installed from a previous version of the package; and the (For example, user installs all rules from Our new I have reproduced it successfully by following the steps mentioned above. You can force the installation of a specific
|
@terrancedejesus Also, just wanted to confirm that I debugged locally using the procedure described above and our endpoint returned 500 when trying to calculate the diff for the 4 "offending" rules listed above (and no others). |
@jpdjere - So the bug exists only on calculating the diff with rules that have changed rule types? And to re-create this we have to install an older package with the previous rule type, then upgrade to receive a package with a different rule type in 8.9+ Are there any drawbacks to adjusting the feature that calculates the diff to just accept rule type changes? Rather than remove packages, re-lock and release rules and re-introduce some noisy rules? |
Just for additional information, when rule tunings are done those changes backport to the least compatible/supported stack in Detection Rules. So if a rule type was changed, that change backports from, for example, main->8.3. When we do our bi-weekly releases we release current-3 meaning we checkout each Detection Rules branch for 8.5, 8.6, 8.7, 8.8, build a package and push that out to EPR. The rule type change will then be present in every single OOB package to these stack versions. Not to mention, it will exist in older OOB packages for those stack versions as well. Since the bug exists in the update workflow involving a new feature in 8.9, removing the These packages are already in customer environments with these rule type changes in use. To better understand where all these rule type diffs exist, I will spend some time reviewing each package from 8.4->8.8 to determine which packages have these rules and which rule types are present for each. This should help us identify the potential exposure of this bug. From the AET side, would it be possible to explore any options available there that would maybe provide a stop-gap until 8.10 or further? |
@jpdjere - Here is the breakout of rule type changes for these offenders across the last 31 ga package releases back to 8.5. <= 8.4 stack versions had the original rule type, therefore I did not include these as they are not affected. All of these rules can be found in OOB packages back to 8.0 stack versions. As you stated, if a user upgrades from 8.0 to 8.9 and the rule types differ, the bug would appear. https://whimsical.com/rule-type-bug-7YjjjK2RS5SoS4oaRijc6L Based on the information in the diagram. removing all packages is not ideal for this. On the flip side, if we remove the |
We've had several discussions previously about the restrictions on changing rule types. As I understand it, these were introduced because we hadn't implemented that capability in the designs for the new workflow. That being said, it doesn't appear to be technically challenging to implement. I'd suggest removing the rule type immutability restriction, rather than forcing a rule deprecation workflow to handle rule type changes. On top of that, removing invariants in the diff calculation algorithm (which is mostly unused in 8.9) seems much simpler than rolling back the rules package release. |
Thanks @terrancedejesus for your work fleshing these changes out and helping us figure this out quickly. Given the situation, we have decided to allow rule type changes in our update diff algorithm. This will increase complexity for us as we develop the algorithm to allow users to view diffs and customize their rules while updating them, but it's the only way we can prevent users from hitting this bug when updating to 8.9 (even if we release patches for all previous minor versions, we cannot force users to update all their "offending" rules before migrating). This change will mean expanding our BE logic in the upgrade diffing algorithm to figuring out how to handle fields that have been removed or added during rule type changes, and in the FE, figuring out how to display these changes and how (or if) to allow the user to customize them. The immediate work to do now to get rid of the issue is:
cc @banderror @xcrzx |
Thanks @jpdjere @terrancedejesus @xcrzx for finding the path forward. |
We're working on a fix in this PR: #161247 |
…kflow (#161247) **Fixes: #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.](#161247 (comment)) 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 <jpdjeredjian@gmail.com>
…kflow (elastic#161247) **Fixes: elastic#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.](elastic#161247 (comment)) 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 <jpdjeredjian@gmail.com> (cherry picked from commit 9e52f70)
…de workflow (#161247) (#161304) # Backport This will backport the following commits from `main` to `8.9`: - [[Security Solution] Support rule type changes in the rule upgrade workflow (#161247)](#161247) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Georgii Gorbachev","email":"georgii.gorbachev@elastic.co"},"sourceCommit":{"committedDate":"2023-07-05T20:42:21Z","message":"[Security Solution] Support rule type changes in the rule upgrade workflow (#161247)\n\n**Fixes: https://github.com/elastic/kibana/issues/161094**\r\n\r\n## Summary\r\n\r\n- Adds support for rule type changes in the\r\n`/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint.\r\n- Previously, if any rule had a different `type` in its\r\n`current_version` compared to its `target_version` the request would\r\nfail with `500`.\r\n- This PR:\r\n - updates this behaviour to accept rule type changes\r\n- creates a new `calculateAllFieldsDiff` method that is responsible for\r\ncalculating diffs among all fields of all rule types. Used exclusively\r\nwhen there has been a rule type change between the current version and\r\nthe target version (which can normally happen through upgrades of the\r\n`security_detection_engine` package) OR when the base version has a\r\ndifferent type as the current version (which should not happen under\r\nnormal conditions and user behaviour).\r\n- updates the diffable fields types for each specifc rule type (e.g.:\r\n`DiffableCustomQueryFields`,`DiffableEqlFields`,`DiffableThreatMatchFields`,\r\netc) , replacing the `data_query` field name for either `eql_query` (for\r\nEQL type rules) or `kql_query` (for all others).\r\n\r\n\r\n## How to test\r\n1. With a clean Kibana state, use the\r\n`xpack.securitySolution.prebuiltRulesPackageVersion` config to force\r\nKibana to install a package that contains the rules with their original\r\ntype:\r\n```\r\nxpack.securitySolution.prebuiltRulesPackageVersion: '8.3.1'\r\n```\r\n2. Install the four \"offending\" rules, [listed\r\nbelow.](https://github.com/elastic/kibana/pull/161247#issuecomment-1622132120)\r\n3. Remove the config, restart Kibana and navigate to the Rules Page so\r\nthat the latest package is installed.\r\n4. Navigate to the Rule Updates table. The four installed rules should\r\nhave updates available. Update them.\r\n5. All the listed rule types should be updated, as well as their\r\ncorresponding fields.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: jpdjere <jpdjeredjian@gmail.com>","sha":"9e52f7064f4016b9c52606ab865149d6b48f5ae4","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","impact:critical","Team:Detections and Resp","Team: SecuritySolution","Team:Detection Rule Management","Feature:Prebuilt Detection Rules","v8.9.0","v8.10.0"],"number":161247,"url":"https://github.com/elastic/kibana/pull/161247","mergeCommit":{"message":"[Security Solution] Support rule type changes in the rule upgrade workflow (#161247)\n\n**Fixes: https://github.com/elastic/kibana/issues/161094**\r\n\r\n## Summary\r\n\r\n- Adds support for rule type changes in the\r\n`/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint.\r\n- Previously, if any rule had a different `type` in its\r\n`current_version` compared to its `target_version` the request would\r\nfail with `500`.\r\n- This PR:\r\n - updates this behaviour to accept rule type changes\r\n- creates a new `calculateAllFieldsDiff` method that is responsible for\r\ncalculating diffs among all fields of all rule types. Used exclusively\r\nwhen there has been a rule type change between the current version and\r\nthe target version (which can normally happen through upgrades of the\r\n`security_detection_engine` package) OR when the base version has a\r\ndifferent type as the current version (which should not happen under\r\nnormal conditions and user behaviour).\r\n- updates the diffable fields types for each specifc rule type (e.g.:\r\n`DiffableCustomQueryFields`,`DiffableEqlFields`,`DiffableThreatMatchFields`,\r\netc) , replacing the `data_query` field name for either `eql_query` (for\r\nEQL type rules) or `kql_query` (for all others).\r\n\r\n\r\n## How to test\r\n1. With a clean Kibana state, use the\r\n`xpack.securitySolution.prebuiltRulesPackageVersion` config to force\r\nKibana to install a package that contains the rules with their original\r\ntype:\r\n```\r\nxpack.securitySolution.prebuiltRulesPackageVersion: '8.3.1'\r\n```\r\n2. Install the four \"offending\" rules, [listed\r\nbelow.](https://github.com/elastic/kibana/pull/161247#issuecomment-1622132120)\r\n3. Remove the config, restart Kibana and navigate to the Rules Page so\r\nthat the latest package is installed.\r\n4. Navigate to the Rule Updates table. The four installed rules should\r\nhave updates available. Update them.\r\n5. All the listed rule types should be updated, as well as their\r\ncorresponding fields.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: jpdjere <jpdjeredjian@gmail.com>","sha":"9e52f7064f4016b9c52606ab865149d6b48f5ae4"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161247","number":161247,"mergeCommit":{"message":"[Security Solution] Support rule type changes in the rule upgrade workflow (#161247)\n\n**Fixes: https://github.com/elastic/kibana/issues/161094**\r\n\r\n## Summary\r\n\r\n- Adds support for rule type changes in the\r\n`/internal/detection_engine/prebuilt_rules/upgrade/_review` endpoint.\r\n- Previously, if any rule had a different `type` in its\r\n`current_version` compared to its `target_version` the request would\r\nfail with `500`.\r\n- This PR:\r\n - updates this behaviour to accept rule type changes\r\n- creates a new `calculateAllFieldsDiff` method that is responsible for\r\ncalculating diffs among all fields of all rule types. Used exclusively\r\nwhen there has been a rule type change between the current version and\r\nthe target version (which can normally happen through upgrades of the\r\n`security_detection_engine` package) OR when the base version has a\r\ndifferent type as the current version (which should not happen under\r\nnormal conditions and user behaviour).\r\n- updates the diffable fields types for each specifc rule type (e.g.:\r\n`DiffableCustomQueryFields`,`DiffableEqlFields`,`DiffableThreatMatchFields`,\r\netc) , replacing the `data_query` field name for either `eql_query` (for\r\nEQL type rules) or `kql_query` (for all others).\r\n\r\n\r\n## How to test\r\n1. With a clean Kibana state, use the\r\n`xpack.securitySolution.prebuiltRulesPackageVersion` config to force\r\nKibana to install a package that contains the rules with their original\r\ntype:\r\n```\r\nxpack.securitySolution.prebuiltRulesPackageVersion: '8.3.1'\r\n```\r\n2. Install the four \"offending\" rules, [listed\r\nbelow.](https://github.com/elastic/kibana/pull/161247#issuecomment-1622132120)\r\n3. Remove the config, restart Kibana and navigate to the Rules Page so\r\nthat the latest package is installed.\r\n4. Navigate to the Rule Updates table. The four installed rules should\r\nhave updates available. Update them.\r\n5. All the listed rule types should be updated, as well as their\r\ncorresponding fields.\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: jpdjere <jpdjeredjian@gmail.com>","sha":"9e52f7064f4016b9c52606ab865149d6b48f5ae4"}}]}] BACKPORT--> Co-authored-by: Georgii Gorbachev <georgii.gorbachev@elastic.co>
@vgomez-el Could you please make sure the fix is verified by the team in the next BC? |
I have upgraded the prebuilt rules package from a 8.3.3 version to a 8.9.2 on 8.9 BC4 version successfully, and I haven't faced the bug. So I can validate the fix. |
🚨🚨🚨 This is a blocker for the
8.9.0
release 🚨🚨🚨I'm not escalating for now, hoping that we can fix it quickly.
Summary
It looks like when the 8.9.1 package with prebuilt rules was released, it broke our rule upgrade workflow. The Rule Updates table doesn't work anymore and doesn't show rules that can be upgraded:
Rule type updates are not supported
The error says
Target version can't be of different rule type
. This error means that in a newly installedsecurity_detection_engine
package there is a rule version in which the rule's type has been changed to a different one (e.g. from Custom query to EQL). In our new API endpoints for the rule upgrade workflow we don't support rule type updates, so in this case these endpoints return the error.kibana/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts
Lines 40 to 41 in 66fd6eb
kibana/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/diff/calculation/calculate_rule_fields_diff.ts
Lines 56 to 66 in 66fd6eb
Proposal for fixing
The most important thing to do is to remove the
8.9.1
version of thesecurity_detection_engine
package from EPR as soon as possible. Since8.9.0
is not yet released, no users in production should have it installed, so it should be safe to remove it. Also, changes introduced by elastic/integrations#6760 (and maybe other related changes, e.g. to the rule assets) will need to be reverted.Then, we will need to revert rule type changes in the source detection-rules repo.
Then, a new
8.9.2
prerelease package needs to be built, pushed to EPR, and tested against Kibana by opening a PR in the kibana repo according to the release process. @jpdjere and @banderror will need to test this package in Kibana manually as well.When the CI passes against the prerelease package, a release
8.9.2
package will need to be published in EPR.After that, we will need to manually double-check that the
8.9.2
release package doesn't break the upgrade process in Kibana.Then, new patch package versions will need to be released for older stacks. These patch packages must roll back rule type changes introduced in the latest patch versions:
I believe the patch package versions to build are:
8.8.7
,8.7.9
,8.6.9
,8.5.9
.The text was updated successfully, but these errors were encountered: