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][Detections] Unexpected behaviors of saved queries in detection rules #76592

Closed
spong opened this issue Sep 3, 2020 · 8 comments
Labels
8.5 candidate bug Fixes for quality problems that affect the customer experience Feature:Detection Rules Anything related to Security Solution's Detection Rules impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. sdh-linked Team:Detection Alerts Security Detection Alerts Area Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM

Comments

@spong
Copy link
Member

spong commented Sep 3, 2020

Summary

This is a meta bug capturing several inconsistent/undesirable behaviors related to use of saved queries in detection rules.

1. Saved query filters referencing missing index patterns automatically disable on Rule edit

Saved query filters created outside of the Security App will be created with a reference to a specific index. This is used internally by the SearchBar component throughout all Kibana apps for fetching field/value autocomplete suggestions. For example here is the same saved query filter created in Discover, Dashboard, and the Security app.

Discover

Dashboard

Security

Now where this becomes an issue is if you create a Rule that uses a saved query with an index reference, and then either delete the corresponding Kibana Index Pattern/Saved Query, or export and import into a space that does not also have this Kibana Index Pattern/Saved Query.

At this point, it should be noted that the rule will continue to run without error so long as it isn't edited in the UI. Once the rule is edited via the UI, the SearchBar component will validate the filter against the missing index, which will put the filter in an error state, and once the Rule is then saved the filter will be saved with disabled: true, thus disabling the filter, and resulting in subsequent Rule runs to execute without the filter (so long as the Saved Query is deleted as well).

2. Inability to clear a saved query in the UI

While it may remove the querystring/filter, the saved_id is not cleared and still references the selected saved query:

3. The Rule Details page doesn't show disabled filter modifications

4. The Rule Details / Edit Rules do not show the query that will be executed

These pages show the query/filter saved to the Rule object and doesn't query for the latest saved query values as what happens during Rule execution, so there could be a mis-match in what is displayed to the user if the saved query is later updated.

5. No insight into applied filters

When there is a saved_id the DE will always query for the latest saved query, and if not found fall back to the querystring/filters on the Rule object. For either scenario, no indication of which filters were used are saved to the resulting alerts -- the system will always copy over the querystring/filters on the Rule object to signal.rule.query/filters.

@spong spong added bug Fixes for quality problems that affect the customer experience Team:SIEM Feature:Detection Rules Anything related to Security Solution's Detection Rules labels Sep 3, 2020
@spong spong self-assigned this Sep 3, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@FrankHassanabad
Copy link
Contributor

FrankHassanabad commented Sep 3, 2020

Other observations from yesterday and more testing with other people that might need other tickets, or this one tagged with more components:

  • The discover page will hot reload the saved query on page load when it sees that there is a saved_id on the query.
  • The discover page does not look to have a poller/observable to re-load the saved_id but rather just reloads it when there is a page load.
  • The discover page will put up an ERROR toaster when it sees that the saved_id is gone if someone deletes the saved query and if it sees that it will "clear" both the text string query and the filters.

Screen Shot 2020-09-03 at 9 55 11 AM

Our security solutions global query bar and our timeline query bar does not do a hot reload when it sees the page being reloaded and detects a saved query when the saved_id is set on the query. Instead both hydrate either from saved object state, local state, url state, in-memory, etc... However, the global security solutions search bar clears the string but not the filter when it detects that the saved query is gone so it half way hydrates it looks like when the saved query is deleted.

Screen Shot 2020-09-03 at 9 57 28 AM

When you click "clear" on discover and on our security solutions global search query bar to remove a saved search, they both will clear both the text string and the filters. Our global security solution query bar clears both the string and filter out like discover does.

Timeline only clears the filters but keeps its query string when the "clear" button is clicked:
Screen Shot 2020-09-03 at 9 59 17 AM

And as you pointed out, the DE engine doesn't clear anything and is "stuck".

I think the consensus (so far, this is all debatable here so just throwing some things out) is that the security solution global query bar and the timeline query bar should both be consistent with discover and both should:

  • Clear the filters and the string in the search box when clear button is clicked.
  • Both the security solutions global search and timeline should hot-reload the saved id directly in the UI when it hydrates from the backend.

However, what if that saved_id is now deleted? I think the thoughts so far there is that both the security application query and the timeline query should do the same thing as discover which is looking like they should clear out the filters and search string and put up a similar/same error toaster.

But note, when you share a URL and that saved_id is gone then you are going to get an error toaster and clean slate meaning you might do an empty query of everything for the timeframe. However this change would fix one other side effect we noticed which is:

  • When moving from discover with a saved query selected to the security solution application right now it can look like our security solutions is dropping filters because it does not do a hot-reload of the saved query when it sees the "saved_id" but rather it is rehydrating parts from another area such as URL, local state, in-memory, etc... instead

So if during the route change if we hot reload the saved query ("saved_id") then it will look like it is in sync with discover even though you have not chosen the "pin" option which I think it will be the behavior users expect which is when they use one UI screen to modify a saved search, and route travel to the next part of the application your filters and search string would follow. There might be a flash of old state as the async call is made to the back end though to check for changes (at worst case if it cannot read it from URL state, local state, etc...). If another user changes the saved search in the meantime, you suddenly will get the new changes from that user loaded as well.

One other note is that:

There is not OCC (Optimistic Concurrency Control) for the saved queries so everything with saved queries on discover is last one in wins. If you have multiple tabs open and try to save a query but in the meantime that query has already been modified your changes will automatically overwrite without warnings the newer change set rather than block the new change set to let the other user know that the saved query has been changed since they first loaded it.

This might influence why you would want to try and hot-reload on route changes/travels and timeline reloads when you see the saved_id because otherwise you risk users coming from very old timelines, page travels, etc... overwriting and changing the last saved query or becoming confused about "why they don't see their changes to their saved queries". This can be particularly bad if they are using very different saved queries and load a timeline from the past which is hydrating a very old saved query and then they re-save and overwrite the much newer work.

@spong
Copy link
Member Author

spong commented Sep 15, 2020

For this core issue, we'll be removing the attach as reference functionality of saved queries so that it matches the behavior of the load query from saved Timeline feature (one time load of query on rule creation). This essentially removes the need for the saved_id field, and as a result, could change the behavior of existing rules on when users update, so we'll need to call this out in the update documentation (cc @benskelker -- will ping you on the fix as well with specific details). As part of this we'll also need to add a check when loading filters to delete the index field if present to prevent the disabled filter behavior described above.

@MadameSheema MadameSheema added the Team:Detections and Resp Security Detection Response Team label Oct 1, 2020
@spong spong removed their assignment Oct 1, 2020
@spong spong added v7.11.0 and removed v7.10.0 labels Oct 1, 2020
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security Solution)

@peluja1012 peluja1012 added the impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. label Oct 28, 2020
@spong spong added impact:critical This issue should be addressed immediately due to a critical level of impact on the product. and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels Feb 24, 2021
@spong
Copy link
Member Author

spong commented Feb 24, 2021

This issue has again been reported within some users rule management workflows -- raising impact so we can prevent future disruptions here.

@MadameSheema MadameSheema added impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:critical This issue should be addressed immediately due to a critical level of impact on the product. labels Mar 4, 2021
@MadameSheema MadameSheema added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. and removed impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. v7.13.0 labels Apr 8, 2021
@rylnd rylnd changed the title [Security Solution][Detections] Saved query filters referencing missing index patterns automatically disable on Rule edit [Security Solution][Detections] Unexpected behaviors of saved queries in detection rules Aug 19, 2021
@rylnd
Copy link
Contributor

rylnd commented Aug 19, 2021

Related: #109253, #107986

@peluja1012 peluja1012 added Team:Detection Alerts Security Detection Alerts Area Team sdh-linked labels Sep 14, 2021
@nkhristinin
Copy link
Contributor

Update that with this PR merged we will ignore savedQuery for Threshold and Indicator Match, and just use the custom query from rule configuration.

So, when you create/edit these rules, you can use a saved query, but it just copies the query value into a custom query and then will ignore saved_id.

@marshallmain
Copy link
Contributor

marshallmain commented Sep 21, 2022

Update: with the merging of this PR, the saved query behaviors have been updated to resolve most of the issues described in this issue.

  1. I thought I saw the filter get automatically disabled after deleting the data view referenced by a saved query and going to the edit rule page, but when I tried again to write down the exact steps it did not reproduce. Further investigation here is warranted to determine if there are cases where the UI can automatically disable the filter. Expected behavior is that if you click "Edit rule" then immediately save the rule again, there should be no changes to the rule.
  2. The linked PR makes it so that the query/filters can't be edited when the rule is referencing a saved query ID. There is also a "Clear All" option now in the saved query menu.
  3. The rule details page still does not show any indication that disabled filters are in fact disabled. ([Security Solution] Disabled rule filters are displayed as enabled in rule details #141458)
  4. The rule details page does now show the filters and query that will be executed 🚀! except for the disabled filters issue in (3)
  5. We still don't have great insight/auditing of the actual query that a rule executed with regards to any dynamically loaded queries, filters, or exceptions.

I will create separate issues for (3) and (5). We should investigate (1) further before finally closing out this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.5 candidate bug Fixes for quality problems that affect the customer experience Feature:Detection Rules Anything related to Security Solution's Detection Rules impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. sdh-linked Team:Detection Alerts Security Detection Alerts Area Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM
Projects
None yet
Development

No branches or pull requests

9 participants