[Security Solution] action not allowed (405) is shown for Duplicating Shared Exception Lists (#177814) by e40pud · Pull Request #178674 · elastic/kibana

e40pud · 2024-03-13T18:31:11Z

Summary

This PR fixes the issue where user is able to import Endpoint lists. Right now endpoint lists (with endpoint_trusted_apps, endpoint_event_filters, endpoint_host_isolation_exceptions or endpoint_blocklists id) are not allowed to be imported. Here we check lists and throw an exception if user tries to import one of the mentioned lists.

However, it is possible to import container endpoint lists with endpoint_list id. This leads to the issue that user can import such a list with the newly generated ID and thus we will treat it as a detection engine list. Since the type of the list is still says endpoint we would not allow to duplicate such a list later here.

To fix the issue, I added addition list id check to prevent users from importing lists with the endpoint_list id.

UPDATE: As discussed below, we will disable the "Create new list" checkbox when user tries to import Endpoint Security Exception List and will show a tooltip saying "We only allow one Exception List for Endpoint Security."

NOTE: as part of this PR, I also added a fix for missing version header in importExceptionList API call.

… Shared Exception Lists (elastic#177814)

elasticmachine · 2024-03-13T18:31:14Z

Pinging @elastic/security-detection-engine (Team:Detection Engine)

elasticmachine · 2024-03-13T18:31:15Z

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

e40pud · 2024-03-13T22:05:33Z

@elasticmachine merge upstream

yctercero · 2024-03-14T05:29:05Z

x-pack/plugins/security_solution/public/exceptions/hooks/use_import_exception_list/index.tsx

  formData.append('file', file as Blob);

  const res = await http.post<ImportExceptionsResponseSchema>(`${EXCEPTION_LIST_URL}/_import`, {
+    version: '2023-10-31',


I think it's worth mentioning in the description that this PR also addresses an issue with a missed version header.

yctercero · 2024-03-14T05:33:55Z

...ecurity_solution/server/lists_integration/endpoint/handlers/exceptions_pre_import_handler.ts

    const hasEndpointArtifactListOrListItems = [...data.lists, ...data.items].some((item) => {
      if ('list_id' in item) {
-        return ALL_ENDPOINT_ARTIFACT_LIST_IDS.includes(item.list_id);
+        return (


@approksiu @elastic/security-defend-workflows just want to confirm this is indeed the desired behavior. I thought I recalled that exception lists of type endpoint were never supposed to be able to be imported as it's considered an "internal" exception list maintained by us.

This fix would mean that users cannot:

Import an exception list of type endpoint

Import rules with exception lists of type endpoint (pretty sure the rule import would succeed but the exception portion would fail)

Pinging @elastic/security-defend-workflows any feedback here?

Thanks for looking into this @yctercero. Indeed this change would disable the ability for importing exception list of type endpoint. While digging around I found that this change that was introduced a while ago is causing the bug that is attached to this PR. That change is gating which list types can be duplicated (including endpoint list type). I'm not sure if that was an intentional change. If it was, then it does make sense with the changes here.

If not then we need to fix the original source of error, i.e. to exclude the endpoint list type from lists that can not be duplicated.

cc @dasansol92

After discussing this with Ash, here are some questions which would be nice to answer in order to understand how to solve this issue:

When we import endpoint list, what is the expected behaviour of the "Overwrite the existing list" and "Create new list"?

Are we allowed to override Endpoint Security Exception List?

If we allow to "Create new list" out of the endpoint list, right now we end-up with a new list which has a list_id not equal to endpoint_list but still has type='endpoint'.

Is there a possibility of newly created endpoint list from step 3 to collide in some way with the original endpoint list?

If we allow to import endpoint list, what operations are allowed on the newly created endpoint list (edit, duplicate etc.)? If duplication is allowed, why would we then disable duplication on original endpoint list?

Thanks for the questions, @e40pud ! I think most of them would be for product to determine. From what I remember way back during initial implementation was that the endpoint security exception list was an internally maintained list that users could add/remove exceptions from, but not delete or modify the parent list at all. If that is still the case, then I would assume that the behaviors for endpoint list would be:

User cannot delete

User cannot duplicate

User can export

User can import and select to override in the sense of "replace the existing items in this list with the items I'm importing" - nothing would be modified of the parent list

This is going off of maybe outdated thinking though (back from initial implementation when there were a lot of restriction around the endpoint list). @caitlinbetz @approksiu can you confirm what the expected behaviors should be for the endpoint exception list?

per conversations with @approksiu - I think we are okay to prevent users from selecting "Create new list" in this scenario. This aligns with existing functionality where we prevent users from duplicating this list. If possible we should make it clear why this option is unavailable for this list.

Thank for the reply @caitlinbetz. Would this work?

I disabled Create new list checkbox and added a tooltip describing the reason.

thanks @e40pud ! Perhaps for now we can say - "We only allow one Exception List for Endpoint Security." to make it clear as to why they cannot create a new list.

e40pud · 2024-03-18T11:43:34Z

@elasticmachine merge upstream

e40pud · 2024-04-02T10:56:24Z

@elasticmachine merge upstream

e40pud · 2024-04-05T08:10:27Z

@elasticmachine merge upstream

e40pud · 2024-04-08T07:56:17Z

@elasticmachine merge upstream

e40pud · 2024-04-10T07:41:53Z

@elasticmachine merge upstream

e40pud · 2024-04-11T09:34:21Z

@elasticmachine merge upstream

e40pud · 2024-04-22T09:11:23Z

@elasticmachine merge upstream

e40pud · 2024-04-22T11:59:06Z

@elasticmachine merge upstream

szwarckonrad

Code LGTM and I don't see any reason why would it interfere with logic from EDR Workflows side. Have a look @ashokaditya though, you know more about artifacts than I do :)

e40pud · 2024-04-24T12:38:31Z

@elasticmachine merge upstream

ashokaditya · 2024-04-24T15:54:46Z

x-pack/plugins/security_solution/public/exceptions/translations/shared_list.ts

+export const IMPORT_EXCEPTION_ENDPOINT_LIST_WARNING = i18n.translate(
+  'xpack.securitySolution.exceptionsTable.importExceptionEndpointListWarning',
+  {
+    defaultMessage: 'We only allow one Exception List for Endpoint Security.',


This text maybe needs a review from the docs team. I think this needs to be rephrased to adhere to writing guidelines

@caitlinbetz @nastasha-solomon any suggestions on how to adjust this message?

Hey, thanks for the ping! Do you have any objections to removing the pronoun (we) in the message? Our current guidelines indicate that "we" is more appropriate in situations when we're taking an action for the user (i.e., helping them) or making a suggestion. In this situation, we're not helping users along. Instead, we're blocking them from doing what they want to do and then telling them that we are the ones blocking them. If you're ok with removing the pronoun, here are a few options to consider:

Multiple exception lists for Endpoint Security are not allowed.

Only one Endpoint Security Exception list is allowed.

Additional exception lists for Endpoint Security are not supported.

Let me know if these work, or if they're not quite what you're looking for.

This looks great! I will go with the first option.

e40pud · 2024-04-25T07:29:49Z

@elasticmachine merge upstream

kibana-ci · 2024-04-25T09:36:58Z

💚 Build Succeeded

Buildkite Build
Commit: 3203945

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	17.3MB	17.3MB	+1.9KB

History

💚 Build #205607 succeeded 7b73003
💚 Build #204956 succeeded 8503eca
💔 Build #204889 failed fdeca73
💚 Build #204362 succeeded 147a21d
💔 Build #203967 failed 950340f
💔 Build #203539 failed d5334b7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @e40pud

e40pud · 2024-04-25T15:37:29Z

💚 All backports created successfully

Status	Branch	Result
✅	8.14

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

Questions ?

Please refer to the Backport tool documentation

… Shared Exception Lists (elastic#177814) (elastic#178674) ## Summary Addresses elastic#177814 This PR fixes the issue where user is able to import Endpoint lists. Right now endpoint lists (with `endpoint_trusted_apps`, `endpoint_event_filters`, `endpoint_host_isolation_exceptions` or `endpoint_blocklists` id) are not allowed to be imported. [Here we check](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_import_handler.ts#L17) lists and throw an exception if user tries to import one of the mentioned lists. However, it is possible to import container endpoint lists with `endpoint_list` id. This leads to the issue that user can import such a list with the newly generated ID and thus we will treat it as a detection engine list. Since the type of the list is still says `endpoint` we would not allow to duplicate such a list later [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/lists/server/services/exception_lists/duplicate_exception_list.ts#L46). To fix the issue, I added addition list id check to prevent users from importing lists with the `endpoint_list` id. **UPDATE**: As discussed below, we will disable the "Create new list" checkbox when user tries to import Endpoint Security Exception List and will show a tooltip saying "We only allow one Exception List for Endpoint Security." **NOTE**: as part of this PR, I also added a fix for missing version header in `importExceptionList` API call. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com> (cherry picked from commit 7cbd396)

…icating Shared Exception Lists (#177814) (#178674) (#181738) # Backport This will backport the following commits from `main` to `8.14`: - [[Security Solution] action not allowed (405) is shown for Duplicating Shared Exception Lists (#177814) (#178674)](#178674)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)

… Shared Exception Lists (elastic#177814) (elastic#178674) ## Summary Addresses elastic#177814 This PR fixes the issue where user is able to import Endpoint lists. Right now endpoint lists (with `endpoint_trusted_apps`, `endpoint_event_filters`, `endpoint_host_isolation_exceptions` or `endpoint_blocklists` id) are not allowed to be imported. [Here we check](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lists_integration/endpoint/handlers/exceptions_pre_import_handler.ts#L17) lists and throw an exception if user tries to import one of the mentioned lists. However, it is possible to import container endpoint lists with `endpoint_list` id. This leads to the issue that user can import such a list with the newly generated ID and thus we will treat it as a detection engine list. Since the type of the list is still says `endpoint` we would not allow to duplicate such a list later [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/lists/server/services/exception_lists/duplicate_exception_list.ts#L46). To fix the issue, I added addition list id check to prevent users from importing lists with the `endpoint_list` id. **UPDATE**: As discussed below, we will disable the "Create new list" checkbox when user tries to import Endpoint Security Exception List and will show a tooltip saying "We only allow one Exception List for Endpoint Security." **NOTE**: as part of this PR, I also added a fix for missing version header in `importExceptionList` API call. --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>

[Security Solution] action not allowed (405) is shown for Duplicating…

9c77449

… Shared Exception Lists (elastic#177814)

e40pud added release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Engine Security Solution Detection Engine Area labels Mar 13, 2024

e40pud self-assigned this Mar 13, 2024

e40pud requested review from a team as code owners March 13, 2024 18:31

e40pud requested review from ashokaditya and szwarckonrad March 13, 2024 18:31

e40pud requested a review from nkhristinin March 13, 2024 18:31

e40pud requested a review from yctercero March 13, 2024 18:31

Merge branch 'main' into security/bugfix/177814-endpoint-lists

04350e9

yctercero reviewed Mar 14, 2024

View reviewed changes

Merge branch 'main' into security/bugfix/177814-endpoint-lists

7f2e3fc

Merge branch 'main' into security/bugfix/177814-endpoint-lists

2ba871c

Merge branch 'main' into security/bugfix/177814-endpoint-lists

589284a

kibanamachine and others added 2 commits April 8, 2024 03:56

Merge branch 'main' into security/bugfix/177814-endpoint-lists

647772d

Merge branch 'main' into security/bugfix/177814-endpoint-lists

190510b

Merge branch 'main' into security/bugfix/177814-endpoint-lists

7a96a49

Merge branch 'main' into security/bugfix/177814-endpoint-lists

66a2fe2

Merge branch 'main' into security/bugfix/177814-endpoint-lists

fdeca73

Merge branch 'main' into security/bugfix/177814-endpoint-lists

8503eca

szwarckonrad approved these changes Apr 23, 2024

View reviewed changes

Merge branch 'main' into security/bugfix/177814-endpoint-lists

7b73003

ashokaditya reviewed Apr 24, 2024

View reviewed changes

kibanamachine and others added 2 commits April 25, 2024 03:29

Merge branch 'main' into security/bugfix/177814-endpoint-lists

adc17e9

Adjust the message to follow guidelines

3203945

e40pud requested a review from ashokaditya April 25, 2024 07:59

ashokaditya approved these changes Apr 25, 2024

View reviewed changes

e40pud merged commit 7cbd396 into elastic:main Apr 25, 2024

kibanamachine added v8.15.0 backport:skip This PR does not require backporting labels Apr 25, 2024

e40pud added v8.14.0 and removed backport:skip This PR does not require backporting v8.14.0 labels Apr 25, 2024

kibanamachine added the backport:skip This PR does not require backporting label Apr 25, 2024

e40pud added v8.14.0 and removed backport:skip This PR does not require backporting labels Apr 25, 2024

e40pud mentioned this pull request Apr 25, 2024

[8.14] [Security Solution] action not allowed (405) is shown for Duplicating Shared Exception Lists (#177814) (#178674) #181738

Merged

kibanamachine mentioned this pull request Apr 25, 2024

[Search] Fix mapping tab breaking docLinks and refresh #181729

Merged

szaffarano mentioned this pull request Jul 12, 2024

[8.14] [Telemetry][Security Solution] Use the proper index to query builtin alerts (#187859) #188233

Closed

Conversation

e40pud commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented Mar 13, 2024

Uh oh!

elasticmachine commented Mar 13, 2024

Uh oh!

e40pud commented Mar 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e40pud commented Mar 18, 2024

Uh oh!

e40pud commented Apr 2, 2024

Uh oh!

e40pud commented Apr 5, 2024

Uh oh!

e40pud commented Apr 8, 2024

Uh oh!

e40pud commented Apr 10, 2024

Uh oh!

e40pud commented Apr 11, 2024

Uh oh!

e40pud commented Apr 22, 2024

Uh oh!

e40pud commented Apr 22, 2024

Uh oh!

szwarckonrad left a comment

Choose a reason for hiding this comment

Uh oh!

e40pud commented Apr 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

e40pud commented Apr 25, 2024

Uh oh!

kibana-ci commented Apr 25, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

History

Uh oh!

e40pud commented Apr 25, 2024

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

e40pud commented Mar 13, 2024 •

edited

Loading