[Security Solutions][Detection Engine] Critical bug where value lists…

… were not operational (#80368) (#80593) ## Summary Fixes bugs to allow users to use value based lists manually. This isn't a first class citizen of the UI at the moment but you can manually add them to the existing UI as long as it's a single index and does not mix ECS threat lists with item lists. Example is upload a list in the file `hosts.txt` and a type of `keyword`: <img width="808" alt="Screen Shot 2020-10-13 at 9 50 58 AM" src="https://user-images.githubusercontent.com/1151048/95893319-0a33bf00-0d45-11eb-9c67-81fe9495d802.png"> Then add it as a threat mapping using: * Index of `.items-${space_id}` such as `.items-default` * Use the mapping field of "keyword" * Use the query of `list_id: ${file_name}` such as `list_id : "hosts.txt"` <img width="808" alt="Screen Shot 2020-10-13 at 9 50 58 AM" src="https://user-images.githubusercontent.com/1151048/95893884-8af2bb00-0d45-11eb-9a38-97aef6e1a754.png"> <img width="1065" alt="Screen Shot 2020-10-13 at 11 08 40 AM" src="https://user-images.githubusercontent.com/1151048/95893902-92b25f80-0d45-11eb-84a0-5cf60e8ba0bf.png"> ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
elastic · Oct 14, 2020 · 6d20d82 · 6d20d82
1 parent 6f0358f
commit 6d20d82
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 10 deletions.
diff --git a/x-pack/plugins/lists/server/scripts/lists/files/hosts.txt b/x-pack/plugins/lists/server/scripts/lists/files/hosts.txt
@@ -1,2 +1,3 @@
-kibana
+siem-kibana
+siem-windows
 rock01
diff --git a/...urity_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts b/...urity_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signal.ts
@@ -60,6 +60,7 @@ export const createThreatSignal = async ({
     searchAfter: currentThreatList.hits.hits[currentThreatList.hits.hits.length - 1].sort,
     sortField: undefined,
     sortOrder: undefined,
+    listClient,
   });
 
   const threatFilter = buildThreatMappingFilter({

diff --git a/...rity_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts b/...rity_solution/server/lib/detection_engine/signals/threat_mapping/create_threat_signals.ts
@@ -62,6 +62,7 @@ export const createThreatSignals = async ({
     query: threatQuery,
     language: threatLanguage,
     index: threatIndex,
+    listClient,
     searchAfter: undefined,
     sortField: undefined,
     sortOrder: undefined,

diff --git a/...urity_solution/server/lib/detection_engine/signals/threat_mapping/get_threat_list.test.ts b/...urity_solution/server/lib/detection_engine/signals/threat_mapping/get_threat_list.test.ts
@@ -9,23 +9,83 @@ import { getSortWithTieBreaker } from './get_threat_list';
 describe('get_threat_signals', () => {
   describe('getSortWithTieBreaker', () => {
     test('it should return sort field of just timestamp if given no sort order', () => {
-      const sortOrder = getSortWithTieBreaker({ sortField: undefined, sortOrder: undefined });
+      const sortOrder = getSortWithTieBreaker({
+        sortField: undefined,
+        sortOrder: undefined,
+        index: ['index-123'],
+        listItemIndex: 'list-index-123',
+      });
       expect(sortOrder).toEqual([{ '@timestamp': 'asc' }]);
     });
 
+    test('it should return sort field of just tie_breaker_id if given no sort order for a list item index', () => {
+      const sortOrder = getSortWithTieBreaker({
+        sortField: undefined,
+        sortOrder: undefined,
+        index: ['list-item-index-123'],
+        listItemIndex: 'list-item-index-123',
+      });
+      expect(sortOrder).toEqual([{ tie_breaker_id: 'asc' }]);
+    });
+
     test('it should return sort field of timestamp with asc even if sortOrder is changed as it is hard wired in', () => {
-      const sortOrder = getSortWithTieBreaker({ sortField: undefined, sortOrder: 'desc' });
+      const sortOrder = getSortWithTieBreaker({
+        sortField: undefined,
+        sortOrder: 'desc',
+        index: ['index-123'],
+        listItemIndex: 'list-index-123',
+      });
       expect(sortOrder).toEqual([{ '@timestamp': 'asc' }]);
     });
 
+    test('it should return sort field of tie_breaker_id with asc even if sortOrder is changed as it is hard wired in for a list item index', () => {
+      const sortOrder = getSortWithTieBreaker({
+        sortField: undefined,
+        sortOrder: 'desc',
+        index: ['list-index-123'],
+        listItemIndex: 'list-index-123',
+      });
+      expect(sortOrder).toEqual([{ tie_breaker_id: 'asc' }]);
+    });
+
     test('it should return sort field of an extra field if given one', () => {
-      const sortOrder = getSortWithTieBreaker({ sortField: 'some-field', sortOrder: undefined });
+      const sortOrder = getSortWithTieBreaker({
+        sortField: 'some-field',
+        sortOrder: undefined,
+        index: ['index-123'],
+        listItemIndex: 'list-index-123',
+      });
       expect(sortOrder).toEqual([{ 'some-field': 'asc', '@timestamp': 'asc' }]);
     });
 
+    test('it should return sort field of an extra field if given one for a list item index', () => {
+      const sortOrder = getSortWithTieBreaker({
+        sortField: 'some-field',
+        sortOrder: undefined,
+        index: ['list-index-123'],
+        listItemIndex: 'list-index-123',
+      });
+      expect(sortOrder).toEqual([{ 'some-field': 'asc', tie_breaker_id: 'asc' }]);
+    });
+
     test('it should return sort field of desc if given one', () => {
-      const sortOrder = getSortWithTieBreaker({ sortField: 'some-field', sortOrder: 'desc' });
+      const sortOrder = getSortWithTieBreaker({
+        sortField: 'some-field',
+        sortOrder: 'desc',
+        index: ['index-123'],
+        listItemIndex: 'list-index-123',
+      });
       expect(sortOrder).toEqual([{ 'some-field': 'desc', '@timestamp': 'asc' }]);
     });
+
+    test('it should return sort field of desc if given one for a list item index', () => {
+      const sortOrder = getSortWithTieBreaker({
+        sortField: 'some-field',
+        sortOrder: 'desc',
+        index: ['list-index-123'],
+        listItemIndex: 'list-index-123',
+      });
+      expect(sortOrder).toEqual([{ 'some-field': 'desc', tie_breaker_id: 'asc' }]);
+    });
   });
 });
diff --git a/...s/security_solution/server/lib/detection_engine/signals/threat_mapping/get_threat_list.ts b/...s/security_solution/server/lib/detection_engine/signals/threat_mapping/get_threat_list.ts
@@ -29,6 +29,7 @@ export const getThreatList = async ({
   sortOrder,
   exceptionItems,
   threatFilters,
+  listClient,
 }: GetThreatListOptions): Promise<SearchResponse<ThreatListItem>> => {
   const calculatedPerPage = perPage ?? MAX_PER_PAGE;
   if (calculatedPerPage > 10000) {
@@ -41,11 +42,17 @@ export const getThreatList = async ({
     index,
     exceptionItems
   );
+
   const response: SearchResponse<ThreatListItem> = await callCluster('search', {
     body: {
       query: queryFilter,
       search_after: searchAfter,
-      sort: getSortWithTieBreaker({ sortField, sortOrder }),
+      sort: getSortWithTieBreaker({
+        sortField,
+        sortOrder,
+        index,
+        listItemIndex: listClient.getListItemIndex(),
+      }),
     },
     ignoreUnavailable: true,
     index,
@@ -54,14 +61,31 @@ export const getThreatList = async ({
   return response;
 };
 
+/**
+ * This returns the sort with a tiebreaker if we find out we are only
+ * querying against the list items index. If we are querying against any
+ * other index we are assuming we are 1 or more ECS compatible indexes and
+ * will query against those indexes using just timestamp since we don't have
+ * a tiebreaker.
+ */
 export const getSortWithTieBreaker = ({
   sortField,
   sortOrder,
+  index,
+  listItemIndex,
 }: GetSortWithTieBreakerOptions): SortWithTieBreaker[] => {
   const ascOrDesc = sortOrder ?? 'asc';
-  if (sortField != null) {
-    return [{ [sortField]: ascOrDesc, '@timestamp': 'asc' }];
+  if (index.length === 1 && index[0] === listItemIndex) {
+    if (sortField != null) {
+      return [{ [sortField]: ascOrDesc, tie_breaker_id: 'asc' }];
+    } else {
+      return [{ tie_breaker_id: 'asc' }];
+    }
   } else {
-    return [{ '@timestamp': 'asc' }];
+    if (sortField != null) {
+      return [{ [sortField]: ascOrDesc, '@timestamp': 'asc' }];
+    } else {
+      return [{ '@timestamp': 'asc' }];
+    }
   }
 };
diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/threat_mapping/types.ts
@@ -155,11 +155,14 @@ export interface GetThreatListOptions {
   sortOrder: 'asc' | 'desc' | undefined;
   threatFilters: PartialFilter[];
   exceptionItems: ExceptionListItemSchema[];
+  listClient: ListClient;
 }
 
 export interface GetSortWithTieBreakerOptions {
   sortField: string | undefined;
   sortOrder: 'asc' | 'desc' | undefined;
+  index: string[];
+  listItemIndex: string;
 }
 
 /**
@@ -171,6 +174,5 @@ export interface ThreatListItem {
 }
 
 export interface SortWithTieBreaker {
-  '@timestamp': 'asc';
   [key: string]: string;
 }