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 Solutions][Detection Engine] Changes wording for threat matches and rules #81334

Merged
merged 9 commits into from
Oct 27, 2020

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Oct 21, 2020

Summary

Changes the wording for threat matches and rules

cc @marrasherrier @MikePaquette @paulewing

Before:

Screen Shot 2020-10-21 at 8 52 44 AM

After:
Screen Shot 2020-10-26 at 10 10 17 PM

Checklist

@FrankHassanabad FrankHassanabad requested review from a team as code owners October 21, 2020 14:51
@FrankHassanabad FrankHassanabad changed the title Changes wording for threat matches and rules [Security Solutions][Detection Engine] Changes wording for threat matches and rules Oct 21, 2020
@FrankHassanabad FrankHassanabad self-assigned this Oct 21, 2020
@FrankHassanabad FrankHassanabad added v8.0.0 v7.10.0 v7.11.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Detection Rules Security Solution rules and Detection Engine Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Oct 21, 2020
@shimonmodi
Copy link

@FrankHassanabad - following up on the updates for threat match rules UI. I have synced with @MikePaquette and we are aligned on there.

  1. Descriptive text "Use indicators from intelligence sources to detect matching events and alerts."
  2. Update tile title to "Indicator Match". Reasoning: we can address more use cases related to watch lists & block lists, by calling it Indicator Match rule type.
  3. Change "Threat Index Patterns" -> "Indicator Index Patterns"
  4. Change "Threat Index Query" -> "Indicator Index Query"
  5. Change "Threat Mapping" -> "Indicator Mapping"

There is also a UI flow change we want to propose. Effectively its a repositioning of existing selections to make it easier to use the feature:

  1. User selects Index Patterns and Threat Index Patterns
  2. Next, User selects Threat Mapping Field (dropdown list) and Threat Index Field (dropdown list).
  3. Next, user can fill in Custom query bar and Threat Index query bar. This would be for advanced users who want to further filter the datasets being matched.
    Reasoning: Most users will only want to specify Threat Mapping Field and Threat Index field as part of the rule definition. There could be advanced users who want to apply additional filters, but we want to avoid starting a user with an advanced rule workflow.
    If this would be better discussed over a quick call let me know.

@FrankHassanabad
Copy link
Contributor Author

FrankHassanabad commented Oct 23, 2020

Next, user can fill in Custom query bar and Threat Index query bar. This would be for advanced users who want to further filter the datasets being matched.

@shimonmodi we currently require the custom query to be filled in for threat matching feature. Should we change that to be defaulted to *:* like we have threat lists so it's not required but rather defaulted when the user selects the card?
Screen Shot 2020-10-22 at 6 01 00 PM

@MikePaquette
Copy link

Thanks @FrankHassanabad those screen shots look good, but there seems to be one more location where threat needs to be changed to indicator as shown here:
image

we currently require the custom query to be filled in for threat matching feature. Should we change that to be defaulted to : like we have threat lists so it's not required but rather defaulted when the user selects the card?

Yes, that would be fine, and should always work, but is a very broad query - guaranteed to return all events.

Could there also be an optimization, with a possible performance improvement, by waiting until the rule author selects the indicator mapping field(s), e.g., source.ip, and then populating the custom query with source.ip: * ? Of course, this would get a bit more complex if ANDs and ORs are specified.

@shimonmodi
Copy link

I would like to evaluate if Mike's suggested optimization is a possible way forward.

@FrankHassanabad
Copy link
Contributor Author

Thanks for the catch on the fields missing a value, I will fix that and update the screenshots!

Yes, that would be fine, and should always work, but is a very broad query - guaranteed to return all events.

Ok, I will let you know if there's anything tricky or weird when I change it as it is a shared component but I think I can manage it without too much hassle.

Could there also be an optimization, with a possible performance improvement, by waiting until the rule author selects the indicator mapping field(s), e.g., source.ip, and then populating the custom query with source.ip: * ? Of course, this would get a bit more complex if ANDs and ORs are specified.

Yeah, that would be bug prone through ambiguity of DSL parsing, but I have good news already. When the filters from the threat list are applied along with any query against the indexes it turns into just one ES translated query that is just as optimal as something like, source.ip: * when it queries the events/logs.

For example if the user has a query of, *:* but and then specified source.ip -> source.ip against their threat mappings and the mappings return a source.ip of say, 127.0.0.1 then it will create a ES query that would become source.ip: 127.0.0.1 which is the exact match we're looking for so the source.ip: * won't add additional perf as we are as efficient as we can be against the large amount of events.

@FrankHassanabad
Copy link
Contributor Author

It turns out that adding a default for the query and changing the UI flow are hard and risky things because of component coupling.

I am going to keep the naming separate from these two work items. I have created a new draft PR that already has the defaulting of the query done here but I don't think we should mix these harder things with renaming for 7.10.0 right now since they are showing to require more code than we want.

New PR for the defaulting of the query when selecting the indicator match:
#81727

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
securitySolution 8.1MB 8.1MB +25.0B

History

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

Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

LGTM! Super duper nit thing on wording. 👍

@@ -1,5 +1,5 @@
{
"name": "Query with a threat mapping",
"name": "Query with a indicator mapping",
Copy link
Contributor

Choose a reason for hiding this comment

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

Super duper nit:

Suggested change
"name": "Query with a indicator mapping",
"name": "Query with an indicator mapping",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crap missed this after the merge! Thanks though, this shows the level you're looking at the code. I appreciate it. Always nervous of making simple slip ups in the UI.

@FrankHassanabad FrankHassanabad merged commit 1066602 into elastic:master Oct 27, 2020
@FrankHassanabad FrankHassanabad deleted the change-wording branch October 27, 2020 18:28
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Oct 27, 2020
…ches and rules (elastic#81334)

## Summary

Changes the wording for threat matches and rules

cc @marrasherrier @MikePaquette @paulewing

Before:

<img width="1063" alt="Screen Shot 2020-10-21 at 8 52 44 AM" src="https://user-images.githubusercontent.com/1151048/96737354-ce1ee080-137a-11eb-973f-6a7d96f69117.png">

After:
<img width="1055" alt="Screen Shot 2020-10-26 at 10 10 17 PM" src="https://user-images.githubusercontent.com/1151048/97256235-1fdec500-17d8-11eb-8a8b-4adffd23dbdc.png">

### Checklist

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Oct 27, 2020
…ches and rules (elastic#81334)

## Summary

Changes the wording for threat matches and rules

cc @marrasherrier @MikePaquette @paulewing

Before:

<img width="1063" alt="Screen Shot 2020-10-21 at 8 52 44 AM" src="https://user-images.githubusercontent.com/1151048/96737354-ce1ee080-137a-11eb-973f-6a7d96f69117.png">

After:
<img width="1055" alt="Screen Shot 2020-10-26 at 10 10 17 PM" src="https://user-images.githubusercontent.com/1151048/97256235-1fdec500-17d8-11eb-8a8b-4adffd23dbdc.png">

### Checklist

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
FrankHassanabad added a commit that referenced this pull request Oct 27, 2020
…ches and rules (#81334) (#81834)

## Summary

Changes the wording for threat matches and rules

cc @marrasherrier @MikePaquette @paulewing

Before:

<img width="1063" alt="Screen Shot 2020-10-21 at 8 52 44 AM" src="https://user-images.githubusercontent.com/1151048/96737354-ce1ee080-137a-11eb-973f-6a7d96f69117.png">

After:
<img width="1055" alt="Screen Shot 2020-10-26 at 10 10 17 PM" src="https://user-images.githubusercontent.com/1151048/97256235-1fdec500-17d8-11eb-8a8b-4adffd23dbdc.png">

### Checklist

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)

Co-authored-by: Kibana Machine <[email protected]>
peluja1012 pushed a commit that referenced this pull request Oct 28, 2020
…eat matches and rules (#81334) (#81835)

* [Security Solutions][Detection Engine] Changes wording for threat matches and rules (#81334)

## Summary

Changes the wording for threat matches and rules

cc @marrasherrier @MikePaquette @paulewing

Before:

<img width="1063" alt="Screen Shot 2020-10-21 at 8 52 44 AM" src="https://user-images.githubusercontent.com/1151048/96737354-ce1ee080-137a-11eb-973f-6a7d96f69117.png">

After:
<img width="1055" alt="Screen Shot 2020-10-26 at 10 10 17 PM" src="https://user-images.githubusercontent.com/1151048/97256235-1fdec500-17d8-11eb-8a8b-4adffd23dbdc.png">

### Checklist

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)

* skips overview tests

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Gloria Hornero <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 28, 2020
…kibana into task-manager/lost-connectivity

* 'task-manager/lost-connectivity' of github.com:gmmorris/kibana:
  skips overview tests (elastic#81877)
  [Security Solution][Case] Fix connector's labeling (elastic#81824)
  [Maps] Fix EMS test (elastic#81856)
  [Security Solutions][Detections] - Fix bug, last response not showing for disabled rules (elastic#81783)
  skip flaky suite (elastic#81853)
  Add tsconfig for url_forwarding (elastic#81177)
  skip flaky suite (elastic#81844)
  check for server enabled (elastic#81818)
  [Seurity Solution][Case] Create case plugin client (elastic#81018)
  [Security Solutions][Detection Engine] Changes wording for threat matches and rules (elastic#81334)
  [Security Solution] critical pref bug with browser fields reducer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Detection Rules Security Solution rules and Detection Engine release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants