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] - Update exceptions logic #71512

Merged
merged 12 commits into from
Jul 15, 2020

Conversation

yctercero
Copy link
Contributor

@yctercero yctercero commented Jul 13, 2020

Summary

This PR addresses a few different issues with how exceptions are handled in the detections engine. We previously were using the following logic:

(rule_query and not exception1) OR (rule_query and not exception2)

This resulted in some confusion in with the logic and expected results. The logic that creates the KQL from the exceptions was updated so that the logic now resembles (visually looks) closer to how the user thinks of it. So given the following:

query & !((item1.entry1 & item1.entry2) || (item2.entry1 & item2.entry2))

...is now being broken down into the following....

query & !((item1.entry1 & item1.entry2) || (item2.entry1 & item2.entry2))

Due to a few limitations, primarily the lack of subquery functionality, we want to find an optimal way of querying the raw events. We've found that we can leverage KQL/ES with exceptions that do not include large value lists as shown above. Any exception items that include large value lists will get dealt with in filter_with_lists.ts.

For context, the exception items are formed using KQL to allow a user to still add a nested exception even when the rule language is lucene. Lucene does not support nested queries, but we are able to send a mix of kql and lucene queries to be built into the proper ES dsl.

Single exception item w/ single entry

Rule Query: host.name: *
KQL: [{"query":"host.name:*", "language":"kuery"}, {"query":"not (event.module:\"traefik\")", "language":"kuery"}]
Exception per UI: event.module - is - traefik
ES query:

{
          query: {
            bool: {
              filter: [
                {
                  bool: {
                    must: [],
                    filter: [
                      {
                        bool: {
                          should: [{ exists: { field: 'host.name' } }],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          must_not: {
                            bool: {
                              should: [{ match_phrase: { 'event.module': 'traefik' } }],
                              minimum_should_match: 1,
                            },
                          },
                        },
                      },
                    ],
                    should: [],
                    must_not: [],
                  },
                },
                {
                  bool: {
                    filter: [
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { gte: '2020-07-14T23:14:51.398Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { lte: '2020-07-14T23:16:21.398Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                    ],
                  },
                },
                { match_all: {} },
              ],
            },
          },
          sort: [{ '@timestamp': { order: 'asc' } }],
       }

Screen Shot 2020-07-14 at 3 48 14 PM

Multiple exception items w/ single entry

Rule Query: host.name: *
KQL: [{"query":"host.name:*","language":"kuery"},{"query":"not ((event.module:\"traefik\") or (source.port:*))","language":"kuery"}]
Exception 1 per UI: event.module - is - traefik
Exception 2 per UI: source.port - exists
ES query:

{
          query: {
            bool: {
              filter: [
                {
                  bool: {
                    must: [],
                    filter: [
                      {
                        bool: {
                          should: [{ exists: { field: 'host.name' } }],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          must_not: {
                            bool: {
                              should: [{ match_phrase: { 'event.module': 'traefik' } }],
                              minimum_should_match: 1,
                            },
                          },
                        },
                      },
                      {
                        bool: {
                          must_not: {
                            bool: {
                              should: [{ exists: { field: 'source.port' } }],
                              minimum_should_match: 1,
                            },
                          },
                        },
                      },
                    ],
                    should: [],
                    must_not: [],
                  },
                },
                {
                  bool: {
                    filter: [
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { gte: '2020-07-14T23:20:18.206Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { lte: '2020-07-14T23:21:48.206Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                    ],
                  },
                },
                { match_all: {} },
              ],
            },
          },
          sort: [{ '@timestamp': { order: 'asc' } }],
        }

Screen Shot 2020-07-14 at 4 02 25 PM

Single exception item w/ multiple entries

Rule Query: host.name: *
KQL: [{"query":"host.name:*","language":"kuery"},{"query":"not (event.module:\"suricata\" and destination.ip:\"10.128.0.33\")","language":"kuery"}]
Exception per UI: event.module - is - suricata and destination.ip - is - 10.128.0.33
ES query:

{
          query: {
            bool: {
              filter: [
                {
                  bool: {
                    must: [],
                    filter: [
                      {
                        bool: {
                          should: [{ exists: { field: 'host.name' } }],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          must_not: {
                            bool: {
                              filter: [
                                {
                                  bool: {
                                    should: [{ match_phrase: { 'event.module': 'suricata' } }],
                                    minimum_should_match: 1,
                                  },
                                },
                                {
                                  bool: {
                                    should: [{ match_phrase: { 'destination.ip': '10.128.0.33' } }],
                                    minimum_should_match: 1,
                                  },
                                },
                              ],
                            },
                          },
                        },
                      },
                    ],
                    should: [],
                    must_not: [],
                  },
                },
                {
                  bool: {
                    filter: [
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { gte: '2020-07-14T20:38:54.668Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                      {
                        bool: {
                          should: [
                            { range: { '@timestamp': { lte: '2020-07-14T20:40:24.668Z' } } },
                          ],
                          minimum_should_match: 1,
                        },
                      },
                    ],
                  },
                },
                { match_all: {} },
              ],
            },
          },
          sort: [{ '@timestamp': { order: 'asc' } }],
        }

Screen Shot 2020-07-14 at 4 40 29 PM

Single exception item w/ large value list entry

Rule Query: host.name: *
KQL: [{"query":"host.name: * ","language":"kuery"}
Post initial KQL search filters candidate signals against large value list
Exception per UI: source.ip - is not in list -[LIST W/ ONE IP]
ES query:

{"query":{"bool":{"filter":[{"bool":{"must":[],"filter":[{"bool":{"should":[{"exists":{"field":"host.name"}}],"minimum_should_match":1}},{"bool":{"should":[{"match_phrase":{"url.path":"/.reporting-*/_search"}}],"minimum_should_match":1}}],"should":[],"must_not":[]}},{"bool":{"filter":[{"bool":{"should":[{"range":{"@timestamp":{"gte":"2020-07-14T06:07:01.505Z"}}}],"minimum_should_match":1}},{"bool":{"should":[{"range":{"@timestamp":{"lte":"2020-07-14T06:22:31.505Z"}}}],"minimum_should_match":1}}]}},{"match_all":{}}]}},"sort":[{"@timestamp":{"order":"asc"}}]}

Screen Shot 2020-07-14 at 2 22 08 AM

Checklist

For maintainers

@dhurley14
Copy link
Contributor

Commenting here but testing this out with the below exceptions yieleded this query (excluding rule query)..

event.module:("traefik") and not source.ip:"69.250.173.110"

Screen Shot 2020-07-13 at 11 08 58 PM

Looks good!

@@ -37,11 +38,28 @@ export const filterEventsAgainstList = async ({
return eventSearchResult;
}

const exceptionItemsWithLargeValueLists = exceptionsList.reduce<ExceptionListItemSchema[]>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: I found that when an exception item did not include a large value list, it still entered the logic of mapping through the exceptions to find large value lists and resulted in filteredHitsPromises being [[]]. Since it read it as not null, and having a length, we would pass back a SignalSearchResponse with 0 hits and this was passed to the function creating the signals.

@tsg
Copy link
Contributor

tsg commented Jul 14, 2020

I have reviewed the DSL from the PR description and they look good.

I have one general comment: the examples here tend to use exceptions that ar in the form of source.ip - is not - 67.173.227.94. While that's possible, it's going to be more likely that users use exceptions that are in the form of source.ip - is - 67.173.227.94. In other words, I expect that the exceptions will be specific in what they exclude. So I think it would be good to have some examples like that.

@tsg
Copy link
Contributor

tsg commented Jul 14, 2020

Single exception item w/ simple and large value list entries
Rule Query: host.name: *
KQL: [{"query":"host.name: * ","language":"kuery"},{"query":"url.path:"/.reporting-/_search"","language":"kuery"}]
Exception per UI: url.path - is not - "/.reporting-
/_search" and source.ip - is not in list -[LIST W/ ONE IP]
ES query:

Ah, I'm not sure about this one.

The logic that we need to implement, on a signal per signal basis, is:

if (url.path - is not - "/.reporting-*/_search" and source.ip - is not in list -[LIST W/ ONE IP]) then ignore

If we put the url.path: "/.reporting-*/_search" part already in the KQL, then it will ignore all signals with that path, regardless of whether the match the second condition or not. So you might end up implementing:

if (url.path - is not - "/.reporting-*/_search" OR source.ip - is not in list -[LIST W/ ONE IP]) then ignore

It could be that if the exception contains a list, then the whole exception needs to be implemented in code, rather than KQL. Not sure if we can get around that.

@yctercero yctercero marked this pull request as ready for review July 14, 2020 23:07
@yctercero yctercero requested review from a team as code owners July 14, 2020 23:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

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

LGTM pending test updates. I'd like to take a closer look at the buildNested function, but that's for another PR.

@yctercero
Copy link
Contributor Author

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
securitySolution 765 +1 764

async chunks size

id value diff baseline
securitySolution 9.0MB +1.8KB 9.0MB

miscellaneous assets size

id value diff baseline
securitySolution 654.2KB -236.0B 654.4KB
upgradeAssistant 22.6KB -5.0B 22.6KB
total - -241.0B -

History

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

@cnasikas cnasikas merged commit 3c9fa99 into elastic:master Jul 15, 2020
cnasikas pushed a commit to cnasikas/kibana that referenced this pull request Jul 15, 2020
…tic#71512)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 15, 2020
* master:
  [APM] Add error rates to Service Map popovers (elastic#69520)
  [Security Solution][Detection Engine] - Update exceptions logic (elastic#71512)
  [Security Solution] Full screen timeline, Collapse event (elastic#71786)
  [Security Solution][Exception Modal] Create endpoint exception list if it doesn't already exist (elastic#71807)
  [Detection Rules] Add 7.9 rules (elastic#71808)
  [Search] Add telemetry for data plugin search service (elastic#70677)
  Add @elastic/safer-lodash-set as an alternative to lodash.set (elastic#67452)
  [tests] Temporarily skipped to promote snapshot
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 15, 2020
* master:
  [APM] Add error rates to Service Map popovers (elastic#69520)
  [Security Solution][Detection Engine] - Update exceptions logic (elastic#71512)
  [Security Solution] Full screen timeline, Collapse event (elastic#71786)
  [Security Solution][Exception Modal] Create endpoint exception list if it doesn't already exist (elastic#71807)
  [Detection Rules] Add 7.9 rules (elastic#71808)
  [Search] Add telemetry for data plugin search service (elastic#70677)
  Add @elastic/safer-lodash-set as an alternative to lodash.set (elastic#67452)
  [tests] Temporarily skipped to promote snapshot
tsg pushed a commit that referenced this pull request Jul 15, 2020
…) (#71847)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>

Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>
marshallmain pushed a commit to marshallmain/kibana that referenced this pull request Jul 16, 2020
…tic#71512)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>
marshallmain added a commit that referenced this pull request Jul 16, 2020
…) (#72106)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>

Co-authored-by: Yara Tercero <yctercero@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Yara Tercero <yara.tercero@elastic.co>
@yctercero yctercero deleted the de_fixes branch October 14, 2020 12:01
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants