[SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists #72304

dhurley14 · 2020-07-17T16:18:44Z

Summary

This fixes a bug where an exception which includes two value lists "anded" together wouldn't return the right ~~signals~~ alerts.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-07-21T12:53:40Z

Pinging @elastic/siem (Team:SIEM)

x-pack/plugins/lists/server/scripts/lists/new/list_ip_item.json

x-pack/plugins/lists/server/scripts/quick_start.sh

FrankHassanabad · 2020-07-21T15:26:17Z

...lugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.test.ts

@@ -106,6 +125,72 @@ describe('filterEventsAgainstList', () => {
        'ci-badguys.txt'
      );
      expect(res.hits.hits.length).toEqual(2);
+
+      // @ts-ignore


Is it possible for us to use an as cast or better yet an is cast using the io-ts is on this here or do we have to keep the ts-ignore. We really should be more strict about allowing ts-ignore. Also there is ts-expect-error which it supposed to replace the ts-ignore:
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-9.html#-ts-expect-error-comments

it will be better in that builds will fail once it is no longer an error condition, but I prefer we try to not use ts-ignore as much as possible.

the ts-ignore says that .ip is not available on item._source.source

Property 'ip' does not exist on type 'string | number | boolean | object | string[] | number[] | boolean[] | object[]'.

comes from here

kibana/x-pack/plugins/security_solution/server/lib/detection_engine/signals/types.ts

Lines 31 to 49 in a7a2b7c

export type SearchTypes =

| string

| string[]

| number

| number[]

| boolean

| boolean[]

| object

| object[]

| undefined;

export interface SignalSource {

[key: string]: SearchTypes;

'@timestamp': string;

signal?: {

parent: Ancestor;

ancestors: Ancestor[];

};

}

I'm not sure there is a way to change that without making a more explicit structure with every ECS property.

FrankHassanabad · 2020-07-21T15:26:39Z

...lugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.test.ts

+      expect(listClient.getListItemByValues as jest.Mock).toHaveBeenCalledTimes(2);
+      expect(res.hits.hits.length).toEqual(6);
+
+      // @ts-ignore


Same comment as above about the ts-ignore.

FrankHassanabad · 2020-07-21T15:26:53Z

...lugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.test.ts

+
+      // @ts-ignore
+      const ipVals = res.hits.hits.map((item) => item._source.source.ip);
+      expect(['1.1.1.1', '3.3.3.3', '5.5.5.5', '7.7.7.7', '8.8.8.8', '9.9.9.9']).toEqual(ipVals);


Thanks for all the tests though! Much appreciated.

marshallmain

Can you add some tests where there are multiple exception items in the exception list and each item has multiple list-based entries?

x-pack/plugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.ts

…lter with lists

… result set

…ut a value list are passed in to the filter function

…ue lists when appearing in different exception items

…cover these)

marshallmain

A couple of minor things, then LGTM

marshallmain · 2020-07-22T04:54:31Z

...lugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.test.ts

+      expect(['1.1.1.1', '3.3.3.3', '5.5.5.5', '7.7.7.7', '8.8.8.8', '9.9.9.9']).toEqual(ipVals);
+    });
+
+    it('should respond with less items in the list given one exception item with two entries of type list if some values match', async () => {


nit: the description says one exception item but the test has 2 exception items with one entry each

marshallmain · 2020-07-22T05:04:42Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.ts

-          .filter((t): t is EntryList => entriesList.is(t))
-          .map(async (entry) => {
+    const valueListExceptionItems = exceptionsList.filter((listItem: ExceptionListItemSchema) => {
+      return listItem.entries.some((entry) => entriesList.is(entry));


probably want listItem.entries.every here since we can't support items that have both list-based and regular entries

We should also modify the exception list item schema to prevent creation of an item with both list and non-list entries, but that can be a separate PR.

… filter logic should execute

kibanamachine · 2020-07-22T16:24:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2eebbda

Build metrics

✅ unchanged

History

💚 Build #63504 succeeded 2f4be13
💚 Build #63159 succeeded 1bd3adf2993016dc63b77eb724ec9bd2c386e47b
💔 Build #63135 failed 8bb0ec986d445de9d90b52ff68da80a21bb7ba8b
💔 Build #63058 failed bc7256a2a10c18ecf654a80168349a53abf2c5af
💔 Build #62691 failed 16a193bd562dc265af99111d15495b3e8dceb11b

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

…ds" between lists (elastic#72304) * wip - comment and sample json for exceptions * promise.all for OR-ing exception items and quick-start script * logging, added/updated json sample scripts, fixed missing await on filter with lists * WIP * bug fix where two lists when 'anded' together were not filtering down result set * undo changes from testing * fix changes to example json and fixes missed conflict with master * update log message and fix type errors * change log statement and add unit test for when exception items without a value list are passed in to the filter function * fix failing test * update expect on one test and adds a new test to ensure anding of value lists when appearing in different exception items * update test after rebasing with master * properly ands exception item entries together with proper test cases * fix test (log statement tests - need to come up with a better way to cover these) * cleans up json examples * rename test and use 'every' in lieu of 'some' when determining if the filter logic should execute

…se "ands" between lists (#72304) (#72908) * wip - comment and sample json for exceptions * promise.all for OR-ing exception items and quick-start script * logging, added/updated json sample scripts, fixed missing await on filter with lists * WIP * bug fix where two lists when 'anded' together were not filtering down result set * undo changes from testing * fix changes to example json and fixes missed conflict with master * update log message and fix type errors * change log statement and add unit test for when exception items without a value list are passed in to the filter function * fix failing test * update expect on one test and adds a new test to ensure anding of value lists when appearing in different exception items * update test after rebasing with master * properly ands exception item entries together with proper test cases * fix test (log statement tests - need to come up with a better way to cover these) * cleans up json examples * rename test and use 'every' in lieu of 'some' when determining if the filter logic should execute

…se "ands" between lists (#72304) (#72909) * wip - comment and sample json for exceptions * promise.all for OR-ing exception items and quick-start script * logging, added/updated json sample scripts, fixed missing await on filter with lists * WIP * bug fix where two lists when 'anded' together were not filtering down result set * undo changes from testing * fix changes to example json and fixes missed conflict with master * update log message and fix type errors * change log statement and add unit test for when exception items without a value list are passed in to the filter function * fix failing test * update expect on one test and adds a new test to ensure anding of value lists when appearing in different exception items * update test after rebasing with master * properly ands exception item entries together with proper test cases * fix test (log statement tests - need to come up with a better way to cover these) * cleans up json examples * rename test and use 'every' in lieu of 'some' when determining if the filter logic should execute

elasticmachine · 2021-09-23T14:29:41Z

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

dhurley14 force-pushed the lists-and-exceptions branch 2 times, most recently from bc7256a to 8bb0ec9 Compare July 20, 2020 21:26

dhurley14 self-assigned this Jul 21, 2020

dhurley14 added release_note:skip Skip the PR/issue when compiling release notes review Team:SIEM v7.10.0 v7.9.0 v8.0.0 labels Jul 21, 2020

dhurley14 marked this pull request as ready for review July 21, 2020 12:53

dhurley14 requested review from a team as code owners July 21, 2020 12:53

FrankHassanabad reviewed Jul 21, 2020

View reviewed changes

x-pack/plugins/lists/server/scripts/lists/new/list_ip_item.json Outdated Show resolved Hide resolved

FrankHassanabad reviewed Jul 21, 2020

View reviewed changes

x-pack/plugins/lists/server/scripts/quick_start.sh Outdated Show resolved Hide resolved

FrankHassanabad reviewed Jul 21, 2020

View reviewed changes

marshallmain reviewed Jul 21, 2020

View reviewed changes

x-pack/plugins/security_solution/server/lib/detection_engine/signals/filter_events_with_list.ts Outdated Show resolved Hide resolved

dhurley14 added 11 commits July 21, 2020 22:39

wip - comment and sample json for exceptions

fe0f3a3

promise.all for OR-ing exception items and quick-start script

a2591c5

logging, added/updated json sample scripts, fixed missing await on fi…

6573981

…lter with lists

WIP

ca537b9

bug fix where two lists when 'anded' together were not filtering down…

09bf9f6

… result set

undo changes from testing

dd8f75d

fix changes to example json and fixes missed conflict with master

f53ca54

update log message and fix type errors

b367024

change log statement and add unit test for when exception items witho…

4d3f524

…ut a value list are passed in to the filter function

fix failing test

051a4f8

update expect on one test and adds a new test to ensure anding of val…

9c5def4

…ue lists when appearing in different exception items

dhurley14 added 3 commits July 21, 2020 22:40

update test after rebasing with master

4efad99

properly ands exception item entries together with proper test cases

7604e01

fix test (log statement tests - need to come up with a better way to …

597fa1d

…cover these)

dhurley14 force-pushed the lists-and-exceptions branch from 4475636 to 597fa1d Compare July 22, 2020 03:12

cleans up json examples

2f4be13

marshallmain approved these changes Jul 22, 2020

View reviewed changes

rename test and use 'every' in lieu of 'some' when determining if the…

2eebbda

… filter logic should execute

dhurley14 merged commit f9cbc99 into elastic:master Jul 22, 2020

dhurley14 deleted the lists-and-exceptions branch July 22, 2020 16:39

dhurley14 mentioned this pull request Jul 22, 2020

[7.x] [SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists (#72304) #72908

Merged

dhurley14 mentioned this pull request Jul 22, 2020

[7.9] [SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists (#72304) #72909

Merged

dhurley14 mentioned this pull request Jul 23, 2020

[Security Solution][Exceptions] Prevents value list entries from co-existing with non value list entries #72995

Merged

1 task

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists #72304

[SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists #72304

dhurley14 commented Jul 17, 2020 •

edited

Loading

elasticmachine commented Jul 21, 2020

FrankHassanabad Jul 21, 2020

dhurley14 Jul 21, 2020

FrankHassanabad Jul 21, 2020

FrankHassanabad Jul 21, 2020

marshallmain left a comment

marshallmain left a comment

marshallmain Jul 22, 2020

marshallmain Jul 22, 2020

marshallmain Jul 22, 2020

kibanamachine commented Jul 22, 2020

elasticmachine commented Sep 23, 2021

	export type SearchTypes =
	\| string
	\| string[]
	\| number
	\| number[]
	\| boolean
	\| boolean[]
	\| object
	\| object[]
	\| undefined;

	export interface SignalSource {
	[key: string]: SearchTypes;
	'@timestamp': string;
	signal?: {
	parent: Ancestor;
	ancestors: Ancestor[];
	};
	}

[SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists #72304

[SIEM] [Detections] Fixes filtering with large value lists to use "ands" between lists #72304

Conversation

dhurley14 commented Jul 17, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jul 21, 2020

FrankHassanabad Jul 21, 2020

Choose a reason for hiding this comment

dhurley14 Jul 21, 2020

Choose a reason for hiding this comment

FrankHassanabad Jul 21, 2020

Choose a reason for hiding this comment

FrankHassanabad Jul 21, 2020

Choose a reason for hiding this comment

marshallmain left a comment

Choose a reason for hiding this comment

marshallmain left a comment

Choose a reason for hiding this comment

marshallmain Jul 22, 2020

Choose a reason for hiding this comment

marshallmain Jul 22, 2020

Choose a reason for hiding this comment

marshallmain Jul 22, 2020

Choose a reason for hiding this comment

kibanamachine commented Jul 22, 2020

💚 Build Succeeded

Build metrics

History

elasticmachine commented Sep 23, 2021

dhurley14 commented Jul 17, 2020 •

edited

Loading