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.

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][Detection Engine] Fix importing rules with multiple types of exception lists #198868

Merged

Conversation

marshallmain
Copy link
Contributor

@marshallmain marshallmain commented Nov 4, 2024

Summary

Fixes #198461

When a rule import file has both single-namespace and namespace-agnostic exception lists, there was a bug in the logic that fetched the existing exception lists after importing them. A missing set of parentheses caused a KQL query that should have read (A OR B) AND (C OR D) to be (A OR B) AND C OR D, meaning that the logic was satisfied by D alone instead of requiring A or B to be true along with D. In this case A and B are filters on exception-list and exception-list-agnostic SO attributes so that we (should) only be looking at the list container objects, i.e. exception-list.attributes.list_type: list. C and D are filters by list_id, e.g. exception-list.attributes.list_id: (test_list_id). Without the extra parentheses around C OR D, the query finds both list and item documents for the list IDs specified in D.

When the findExceptionList logic encounters a list item unexpectedly, it still tries to convert the SO into our internal representation of an exception list with transformSavedObjectToExceptionList. Most fields are shared between lists and items, which makes it confusing to debug. However, the type of items can only be simple, whereas lists have a variety of types. During the conversion, the type field of the resulting object is defaulted to detection if the type field of the SO doesn't match the allowed list type values. Since the related SDH involved importing a rule_default exception list instead, the list types didn't match up when the import route compared the exception list on the rule to import vs the "existing list" (which was actually a list item coerced into a list container schema with type: detection) and import fails.

@marshallmain marshallmain added release_note:skip Skip the PR/issue when compiling release notes sdh-linked v8.17.0 labels Nov 4, 2024
@marshallmain marshallmain requested review from a team as code owners November 4, 2024 22:07
@marshallmain marshallmain added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Nov 4, 2024
@banderror
Copy link
Contributor

@marshallmain Would it be possible to target 8.16.1 and 8.15.5? The user from the linked SDH is on 8.15.

@marshallmain marshallmain added backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 8, 2024
@marshallmain
Copy link
Contributor Author

Yeah targeting those versions shouldn't be an issue, these files haven't changed in a long time so I don't see any conflicts.

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine this one was tricky to track down; nice work finding the root cause.

I had one nit about adding a unit test to demonstrate the importance of the added parentheses, but the integration test implicitly covers that so feel free to ignore.

@@ -60,7 +60,7 @@ describe('getExceptionListFilter', () => {
savedObjectTypes: ['exception-list-agnostic', 'exception-list'],
});
expect(filter).toEqual(
'(exception-list-agnostic.attributes.list_type: list OR exception-list.attributes.list_type: list) AND exception-list-agnostic.attributes.name: "Sample Endpoint Exception List"'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these examples demonstrate the need for the added parentheses; they all look to be logically equivalent. Is it possible to add a test for the situation causing the bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit tests don't actually test the query logic so an extra test would be a near duplicate. I modified this one though to show the importance more clearly.

Copy link
Contributor

@nikitaindik nikitaindik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marshallmain! I've tested the changes locally and can confirm that the issue is fixed.

Here's how I tested:

  • Created a namespace-agnostic exception list via API and added a list item to it
  • Added this namespace-agnostic exception list to a rule
  • Added a single-namespace list item to the same rule
  • Exported the rule
Rule NDJSON
{"id":"e5e5c8d3-61da-4cff-bbdb-28a7e58d819c","updated_at":"2024-11-09T15:42:45.904Z","updated_by":"elastic","created_at":"2024-11-09T15:40:05.325Z","created_by":"elastic","name":"my_rule_2","tags":[],"interval":"5m","enabled":false,"revision":2,"description":".","risk_score":21,"severity":"low","license":"","output_index":"","meta":{"from":"1m","kibana_siem_app_url":"http://localhost:5621/kbn/app/security"},"author":[],"false_positives":[],"from":"now-360s","rule_id":"778d20ac-dd64-46c3-bf20-c3fe7c5661ad","max_signals":100,"risk_score_mapping":[],"severity_mapping":[],"threat":[],"to":"now","references":[],"version":1,"exceptions_list":[{"id":"dd43269c-029c-4f12-a1e8-5e7c993f9002","list_id":"b840d365-9975-4d96-8ccc-a27a6677f13c","type":"rule_default","namespace_type":"single"},{"id":"80fa08df-f08c-4e78-bca6-5990df8f7b6a","list_id":"29e1bb42-18bf-4cba-a448-6ba5a98ebc01","type":"detection","namespace_type":"agnostic"}],"immutable":false,"rule_source":{"type":"internal"},"related_integrations":[],"required_fields":[],"setup":"","type":"query","language":"kuery","index":["apm-*-transaction*","auditbeat-*","endgame-*","filebeat-*","logs-*","packetbeat-*","traces-apm*","winlogbeat-*","-*elastic-cloud-logs-*"],"query":"*","filters":[],"actions":[]}
{"_version":"WzE0NTcsMV0=","created_at":"2024-11-09T15:41:00.769Z","created_by":"elastic","description":"Exception list containing exceptions for rule with id: e5e5c8d3-61da-4cff-bbdb-28a7e58d819c","id":"dd43269c-029c-4f12-a1e8-5e7c993f9002","immutable":false,"list_id":"b840d365-9975-4d96-8ccc-a27a6677f13c","name":"Exceptions for rule - my_rule_2","namespace_type":"single","os_types":[],"tags":["default_rule_exception_list"],"tie_breaker_id":"7ef7627f-c847-4590-9234-897d1a43eb57","type":"rule_default","updated_at":"2024-11-09T15:41:00.769Z","updated_by":"elastic","version":1}
{"_version":"WzE0NTgsMV0=","comments":[],"created_at":"2024-11-09T15:41:02.074Z","created_by":"elastic","description":"Exception list item","entries":[{"type":"exists","field":"a","operator":"included"}],"id":"74a96a81-07e7-4174-a86b-5de5d77f6704","item_id":"0042363e-de7a-48de-97f1-8e01fb863f64","list_id":"b840d365-9975-4d96-8ccc-a27a6677f13c","name":"my-exception","namespace_type":"single","os_types":[],"tags":[],"tie_breaker_id":"0981b732-36a9-4e0d-9ce5-973796728fa0","type":"simple","updated_at":"2024-11-09T15:41:02.074Z","updated_by":"elastic"}
{"_version":"WzE0NTUsMV0=","created_at":"2024-11-09T15:36:09.302Z","created_by":"elastic","description":"string","id":"80fa08df-f08c-4e78-bca6-5990df8f7b6a","immutable":false,"list_id":"29e1bb42-18bf-4cba-a448-6ba5a98ebc01","name":"my-api-exc-l","namespace_type":"agnostic","os_types":[],"tags":[],"tie_breaker_id":"36fd13ae-cf4f-41a5-a2c7-e9f150a408c7","type":"detection","updated_at":"2024-11-09T15:36:09.302Z","updated_by":"elastic","version":1}
{"_version":"WzE0NTYsMV0=","comments":[],"created_at":"2024-11-09T15:37:03.683Z","created_by":"elastic","description":"Exception list item","entries":[{"type":"exists","field":"@timestamp","operator":"included"}],"id":"2e9655f7-2c83-4c76-b227-4f8ca54cd1a4","item_id":"21bd93b6-9560-4298-9114-5540607017aa","list_id":"29e1bb42-18bf-4cba-a448-6ba5a98ebc01","name":"my-exception-item-123","namespace_type":"agnostic","os_types":[],"tags":[],"tie_breaker_id":"bd3a7060-750e-4af3-a52b-4bcd5bcfc7ec","type":"simple","updated_at":"2024-11-09T15:37:03.683Z","updated_by":"elastic"}
{"exported_count":5,"exported_rules_count":1,"missing_rules":[],"missing_rules_count":0,"exported_exception_list_count":2,"exported_exception_list_item_count":2,"missing_exception_list_item_count":0,"missing_exception_list_items":[],"missing_exception_lists":[],"missing_exception_lists_count":0,"exported_action_connector_count":0,"missing_action_connection_count":0,"missing_action_connections":[],"excluded_action_connection_count":0,"excluded_action_connections":[]}
  • Imported it into a clean local Kibana on main - got an error
Scherm­afbeelding 2024-11-09 om 16 48 03
  • Imported it into a clean local Kibana on Marshall's branch - import was successful
Scherm­afbeelding 2024-11-09 om 16 50 25

I’ve also reviewed the code and test changes; everything looks good to me.

@nikitaindik nikitaindik requested a review from xcrzx November 9, 2024 16:11
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #55 / @ess @serverless SecuritySolution Timeline @skipInServerless Timeline migrations 8.0 id migration "before all" hook in "8.0 id migration"

Metrics [docs]

✅ unchanged

History

@marshallmain marshallmain merged commit 0cc2e56 into elastic:main Nov 13, 2024
39 checks passed
@marshallmain marshallmain deleted the exception-import-space-agnostic branch November 13, 2024 20:01
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.15, 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11824878397

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 13, 2024
…le types of exception lists (elastic#198868)

## Summary

Fixes elastic#198461

When a rule import file has both single-namespace and namespace-agnostic
exception lists, there was a bug in the logic that fetched the existing
exception lists after importing them. A missing set of parentheses
caused a KQL query that should have read `(A OR B) AND (C OR D)` to be
`(A OR B) AND C OR D`, meaning that the logic was satisfied by `D` alone
instead of requiring `A` or `B` to be true along with `D`. In this case
`A` and `B` are filters on `exception-list` and
`exception-list-agnostic` SO attributes so that we (should) only be
looking at the list container objects, i.e.
`exception-list.attributes.list_type: list`. `C` and `D` are filters by
`list_id`, e.g. `exception-list.attributes.list_id: (test_list_id)`.
Without the extra parentheses around `C OR D`, the query finds both
`list` and `item` documents for the list IDs specified in `D`.

When the `findExceptionList` logic encounters a list item unexpectedly,
it still tries to convert the SO into our internal representation of an
exception list with `transformSavedObjectToExceptionList`. Most fields
are shared between lists and items, which makes it confusing to debug.
However, the `type` of items can only be `simple`, whereas lists have a
variety of types. During the conversion, the `type` field of the
resulting object is defaulted to `detection` if the `type` field of the
SO doesn't match the allowed list type values. Since the related SDH
involved importing a `rule_default` exception list instead, the list
types didn't match up when the import route compared the exception list
on the rule to import vs the "existing list" (which was actually a list
item coerced into a list container schema with `type: detection`) and
import fails.

(cherry picked from commit 0cc2e56)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 13, 2024
…le types of exception lists (elastic#198868)

## Summary

Fixes elastic#198461

When a rule import file has both single-namespace and namespace-agnostic
exception lists, there was a bug in the logic that fetched the existing
exception lists after importing them. A missing set of parentheses
caused a KQL query that should have read `(A OR B) AND (C OR D)` to be
`(A OR B) AND C OR D`, meaning that the logic was satisfied by `D` alone
instead of requiring `A` or `B` to be true along with `D`. In this case
`A` and `B` are filters on `exception-list` and
`exception-list-agnostic` SO attributes so that we (should) only be
looking at the list container objects, i.e.
`exception-list.attributes.list_type: list`. `C` and `D` are filters by
`list_id`, e.g. `exception-list.attributes.list_id: (test_list_id)`.
Without the extra parentheses around `C OR D`, the query finds both
`list` and `item` documents for the list IDs specified in `D`.

When the `findExceptionList` logic encounters a list item unexpectedly,
it still tries to convert the SO into our internal representation of an
exception list with `transformSavedObjectToExceptionList`. Most fields
are shared between lists and items, which makes it confusing to debug.
However, the `type` of items can only be `simple`, whereas lists have a
variety of types. During the conversion, the `type` field of the
resulting object is defaulted to `detection` if the `type` field of the
SO doesn't match the allowed list type values. Since the related SDH
involved importing a `rule_default` exception list instead, the list
types didn't match up when the import route compared the exception list
on the rule to import vs the "existing list" (which was actually a list
item coerced into a list container schema with `type: detection`) and
import fails.

(cherry picked from commit 0cc2e56)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 13, 2024
…le types of exception lists (elastic#198868)

## Summary

Fixes elastic#198461

When a rule import file has both single-namespace and namespace-agnostic
exception lists, there was a bug in the logic that fetched the existing
exception lists after importing them. A missing set of parentheses
caused a KQL query that should have read `(A OR B) AND (C OR D)` to be
`(A OR B) AND C OR D`, meaning that the logic was satisfied by `D` alone
instead of requiring `A` or `B` to be true along with `D`. In this case
`A` and `B` are filters on `exception-list` and
`exception-list-agnostic` SO attributes so that we (should) only be
looking at the list container objects, i.e.
`exception-list.attributes.list_type: list`. `C` and `D` are filters by
`list_id`, e.g. `exception-list.attributes.list_id: (test_list_id)`.
Without the extra parentheses around `C OR D`, the query finds both
`list` and `item` documents for the list IDs specified in `D`.

When the `findExceptionList` logic encounters a list item unexpectedly,
it still tries to convert the SO into our internal representation of an
exception list with `transformSavedObjectToExceptionList`. Most fields
are shared between lists and items, which makes it confusing to debug.
However, the `type` of items can only be `simple`, whereas lists have a
variety of types. During the conversion, the `type` field of the
resulting object is defaulted to `detection` if the `type` field of the
SO doesn't match the allowed list type values. Since the related SDH
involved importing a `rule_default` exception list instead, the list
types didn't match up when the import route compared the exception list
on the rule to import vs the "existing list" (which was actually a list
item coerced into a list container schema with `type: detection`) and
import fails.

(cherry picked from commit 0cc2e56)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.15
8.16
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 13, 2024
… multiple types of exception lists (#198868) (#200085)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Solution][Detection Engine] Fix importing rules with
multiple types of exception lists
(#198868)](#198868)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-13T20:01:18Z","message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","sdh-linked","backport:prev-major","v8.17.0"],"title":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception
lists","number":198868,"url":"https://github.com/elastic/kibana/pull/198868","mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198868","number":198868,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <[email protected]>
kibanamachine added a commit that referenced this pull request Nov 13, 2024
…multiple types of exception lists (#198868) (#200086)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Detection Engine] Fix importing rules with
multiple types of exception lists
(#198868)](#198868)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-13T20:01:18Z","message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","sdh-linked","backport:prev-major","v8.17.0"],"title":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception
lists","number":198868,"url":"https://github.com/elastic/kibana/pull/198868","mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198868","number":198868,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <[email protected]>
kibanamachine added a commit that referenced this pull request Nov 13, 2024
… multiple types of exception lists (#198868) (#200084)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Security Solution][Detection Engine] Fix importing rules with
multiple types of exception lists
(#198868)](#198868)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marshall
Main","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-13T20:01:18Z","message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","sdh-linked","backport:prev-major","v8.17.0"],"title":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception
lists","number":198868,"url":"https://github.com/elastic/kibana/pull/198868","mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198868","number":198868,"mergeCommit":{"message":"[Security
Solution][Detection Engine] Fix importing rules with multiple types of
exception lists (#198868)\n\n## Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/198461\r\n\r\nWhen a rule
import file has both single-namespace and
namespace-agnostic\r\nexception lists, there was a bug in the logic that
fetched the existing\r\nexception lists after importing them. A missing
set of parentheses\r\ncaused a KQL query that should have read `(A OR B)
AND (C OR D)` to be\r\n`(A OR B) AND C OR D`, meaning that the logic was
satisfied by `D` alone\r\ninstead of requiring `A` or `B` to be true
along with `D`. In this case\r\n`A` and `B` are filters on
`exception-list` and\r\n`exception-list-agnostic` SO attributes so that
we (should) only be\r\nlooking at the list container objects,
i.e.\r\n`exception-list.attributes.list_type: list`. `C` and `D` are
filters by\r\n`list_id`, e.g. `exception-list.attributes.list_id:
(test_list_id)`.\r\nWithout the extra parentheses around `C OR D`, the
query finds both\r\n`list` and `item` documents for the list IDs
specified in `D`.\r\n\r\nWhen the `findExceptionList` logic encounters a
list item unexpectedly,\r\nit still tries to convert the SO into our
internal representation of an\r\nexception list with
`transformSavedObjectToExceptionList`. Most fields\r\nare shared between
lists and items, which makes it confusing to debug.\r\nHowever, the
`type` of items can only be `simple`, whereas lists have a\r\nvariety of
types. During the conversion, the `type` field of the\r\nresulting
object is defaulted to `detection` if the `type` field of the\r\nSO
doesn't match the allowed list type values. Since the related
SDH\r\ninvolved importing a `rule_default` exception list instead, the
list\r\ntypes didn't match up when the import route compared the
exception list\r\non the rule to import vs the \"existing list\" (which
was actually a list\r\nitem coerced into a list container schema with
`type: detection`) and\r\nimport
fails.","sha":"0cc2e5677b46393ffd066ddaa1c548c664af311b"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Marshall Main <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…le types of exception lists (elastic#198868)

## Summary

Fixes elastic#198461

When a rule import file has both single-namespace and namespace-agnostic
exception lists, there was a bug in the logic that fetched the existing
exception lists after importing them. A missing set of parentheses
caused a KQL query that should have read `(A OR B) AND (C OR D)` to be
`(A OR B) AND C OR D`, meaning that the logic was satisfied by `D` alone
instead of requiring `A` or `B` to be true along with `D`. In this case
`A` and `B` are filters on `exception-list` and
`exception-list-agnostic` SO attributes so that we (should) only be
looking at the list container objects, i.e.
`exception-list.attributes.list_type: list`. `C` and `D` are filters by
`list_id`, e.g. `exception-list.attributes.list_id: (test_list_id)`.
Without the extra parentheses around `C OR D`, the query finds both
`list` and `item` documents for the list IDs specified in `D`.

When the `findExceptionList` logic encounters a list item unexpectedly,
it still tries to convert the SO into our internal representation of an
exception list with `transformSavedObjectToExceptionList`. Most fields
are shared between lists and items, which makes it confusing to debug.
However, the `type` of items can only be `simple`, whereas lists have a
variety of types. During the conversion, the `type` field of the
resulting object is defaulted to `detection` if the `type` field of the
SO doesn't match the allowed list type values. Since the related SDH
involved importing a `rule_default` exception list instead, the list
types didn't match up when the import route compared the exception list
on the rule to import vs the "existing list" (which was actually a list
item coerced into a list container schema with `type: detection`) and
import fails.
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
…le types of exception lists (elastic#198868)

## Summary

Fixes elastic#198461

When a rule import file has both single-namespace and namespace-agnostic
exception lists, there was a bug in the logic that fetched the existing
exception lists after importing them. A missing set of parentheses
caused a KQL query that should have read `(A OR B) AND (C OR D)` to be
`(A OR B) AND C OR D`, meaning that the logic was satisfied by `D` alone
instead of requiring `A` or `B` to be true along with `D`. In this case
`A` and `B` are filters on `exception-list` and
`exception-list-agnostic` SO attributes so that we (should) only be
looking at the list container objects, i.e.
`exception-list.attributes.list_type: list`. `C` and `D` are filters by
`list_id`, e.g. `exception-list.attributes.list_id: (test_list_id)`.
Without the extra parentheses around `C OR D`, the query finds both
`list` and `item` documents for the list IDs specified in `D`.

When the `findExceptionList` logic encounters a list item unexpectedly,
it still tries to convert the SO into our internal representation of an
exception list with `transformSavedObjectToExceptionList`. Most fields
are shared between lists and items, which makes it confusing to debug.
However, the `type` of items can only be `simple`, whereas lists have a
variety of types. During the conversion, the `type` field of the
resulting object is defaulted to `detection` if the `type` field of the
SO doesn't match the allowed list type values. Since the related SDH
involved importing a `rule_default` exception list instead, the list
types didn't match up when the import route compared the exception list
on the rule to import vs the "existing list" (which was actually a list
item coerced into a list container schema with `type: detection`) and
import fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-major Backport to (8.x, 8.17, 8.16) the previous major branch and other branches in development release_note:skip Skip the PR/issue when compiling release notes sdh-linked v8.15.5 v8.16.1 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution] Error when importing a duplicated Endpoint Security rule with default exceptions
7 participants