[Security Solution][Resolver] Handle disabled process collection #73592

jonathan-buttner · 2020-07-29T00:18:23Z

This PR allows resolver to gracefully handle events sent by the endpoint that do not have process.entity_id defined or it is set to an empty string.

Notable changes:

The backend will return a bad request if the entity_id is an empty string in the API requests
The resolver icon will only show if process.entity_id exists on the document and it is not an empty string
The backend only retrieves documents that have a non-empty string for entity_id
The backend removes any entity_ids in the queries that are empty strings

entity_id is an empty string

entity_id exists and is not an empty string

elasticmachine · 2020-07-29T00:18:25Z

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

elasticmachine · 2020-07-29T00:18:25Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

michaelolo24 · 2020-07-29T00:22:09Z

x-pack/plugins/security_solution/public/timelines/components/timeline/body/helpers.test.ts

+    });
+
+    it('returns false if process.entity_id is an empty array', () => {
+      const data: Ecs = { _id: '1', agent: { type: ['endpoint'] } };


fyi, this is missing the empty array

Ah thanks. I actually check that case below. Must be an accidental copy paste. I'll remove it.

oatkiller · 2020-07-29T00:23:47Z

x-pack/test/security_solution_endpoint_api_int/apis/resolver/entity_id.ts

+      });
+
+      it('excludes events that have an empty entity_id field', async () => {
+        // first lets get the _id of the document using the parent.process.entity_id


nothing to block over, but can't you set _id when you insert a document?

Yeah good point. I'm using a helper function that inserts an array of documents so it's doing a bulk request. Later we can refactor it so it specifies the _id of the document so we don't have to do the search.

oatkiller · 2020-07-29T00:24:37Z

x-pack/test/security_solution_endpoint_api_int/apis/resolver/entity_id.ts

+      before(async () => {
+        // construct a tree with an origin and two direct children. One child will not have an entity_id. That child
+        // should not be returned by the backend.
+        origin = generator.generateEvent({ entityID: 'a' });


we should add tests of this kind to the UI too eventually

oatkiller · 2020-07-29T00:30:21Z

x-pack/test/security_solution_endpoint_api_int/apis/resolver/entity_id.ts

+        ancestor2 = generator.generateEvent({
+          entityID: '2',
+        });
+        ancestor1 = generator.generateEvent({


having trouble following what ancestor1 is for. but i'm tired. maybe you can explain it to me next week :)

Yeah it was more of a sanity check. The origin node in that test has its process.parent.entity_id set to ancestor1's process.entity_id, but the backend should be using the ancestry array to determine the ancestors if it exists. So since ancestor1 was not returned in the request we can be assured that the backend is properly using the ancestry array.

michaelolo24 · 2020-07-29T00:34:09Z

x-pack/test/security_solution_endpoint_api_int/apis/resolver/entity_id.ts

+        origin = generator.generateEvent({
+          entityID: 'a',
+          parentEntityID: ancestor1.process.entity_id,
+          ancestry: ['', ancestor2.process.entity_id],


you could just make the first one here ancestorNoEntityID.process.entity_id too. Took me a sec to figure that's what was being referenced. Nothing to restart CI over though

Yeah good point. I added that late. I wanted to make sure that the backend wouldn't return the event even if ES actually did have a document with entity_id as an empty string

…ed-process-collection-v2

jonathan-buttner · 2020-07-29T02:17:30Z

@elasticmachine merge upstream

kibanamachine · 2020-07-29T04:01:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 62d55fb

Build metrics

async chunks size

id	value	diff	baseline
securitySolution	7.3MB	+97.0B	7.3MB

History

💔 Build #65052 failed 21eee4f
💔 Build #65037 failed 4801974
💔 Build #65055 failed d33b537

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

…stic#73592) * Handling entity ids of empty string * Tests for entity id being empty * More comments * entity test * Renaming interface * Removing unneeded test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

) (#73619) * Handling entity ids of empty string * Tests for entity id being empty * More comments * entity test * Renaming interface * Removing unneeded test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

#73592) (#73620) * [Security Solution][Resolver] Handle disabled process collection (#73592) * Handling entity ids of empty string * Tests for entity id being empty * More comments * entity test * Renaming interface * Removing unneeded test Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * Fixing type error Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: (126 commits) [ML] Disabling ML if license feature is disabled (elastic#73187) [ML] Fixing old _xpack style es endpoint paths (elastic#73667) [DOCS] [Lens] 7.9 docs refresh (elastic#72301) [ML] DF Analytics results: ensure `View` link is only enabled when job has successfully completed (elastic#73539) Set timeRange to default to trigger the error message (elastic#73629) [ML] Functional tests - stabilize DFA navigation and index pattern handling (elastic#73660) [ILM] Add links to "Snapshot and Restore" from ILM "wait for snapshot policy" (elastic#72473) [kbn-storybook] Update Storybook to 5.3.19 (elastic#73320) [Metrics UI] Fix hasData call to ensure it has data not just indices (elastic#72969) [Uptime] Use `service.name` to link from Uptime -> APM where available (elastic#73618) allow others to update `URL.revokeObjectURL` property if needed (elastic#73639) regen docs (elastic#73650) [Visualize] Fix inspector download filename issue when saving in-place (elastic#72605) [Data] Query Input String manager (elastic#72093) [Security Solutions] Add tooltips (elastic#73436) Do not render descriptionless actions within an EuiCard (elastic#73611) [Security Solution][Detections] Value Lists Modal supports multiple exports (elastic#73532) [Security Solution][Resolver] Handle disabled process collection (elastic#73592) [Security_Solution][Bug] Fix user name/domain to ECS structure (elastic#73530) [Security Solution][Exceptions] - Update rule.exceptions_list to include exception list list_id (elastic#73349) ...

jonathan-buttner added 4 commits July 28, 2020 17:12

Handling entity ids of empty string

9424a64

Tests for entity id being empty

7e3f9d8

More comments

f6f41d2

entity test

37b884d

jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver Feature:Endpoint Elastic Endpoint feature v7.9.0 labels Jul 29, 2020

jonathan-buttner requested review from oatkiller, bkimmel, andrew-goldstein, michaelolo24, james-elastic and kqualters-elastic July 29, 2020 00:18

jonathan-buttner requested review from a team as code owners July 29, 2020 00:18

Renaming interface

4801974

michaelolo24 reviewed Jul 29, 2020

View reviewed changes

Removing unneeded test

21eee4f

oatkiller approved these changes Jul 29, 2020

View reviewed changes

oatkiller reviewed Jul 29, 2020

View reviewed changes

michaelolo24 approved these changes Jul 29, 2020

View reviewed changes

michaelolo24 reviewed Jul 29, 2020

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into handle-disabl…

d33b537

…ed-process-collection-v2

Merge branch 'master' into handle-disabled-process-collection-v2

62d55fb

jonathan-buttner merged commit 41c2967 into elastic:master Jul 29, 2020

jonathan-buttner deleted the handle-disabled-process-collection-v2 branch July 29, 2020 04:01

This was referenced Jul 29, 2020

[7.x] [Security Solution][Resolver] Handle disabled process collection (#73592) #73619

Merged

[7.9] [Security Solution][Resolver] Handle disabled process collection (#73592) #73620

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Resolver] Handle disabled process collection #73592

[Security Solution][Resolver] Handle disabled process collection #73592

jonathan-buttner commented Jul 29, 2020 •

edited

Loading

elasticmachine commented Jul 29, 2020

elasticmachine commented Jul 29, 2020

michaelolo24 Jul 29, 2020

jonathan-buttner Jul 29, 2020

oatkiller Jul 29, 2020

jonathan-buttner Jul 29, 2020

oatkiller Jul 29, 2020

oatkiller Jul 29, 2020

jonathan-buttner Jul 29, 2020

michaelolo24 Jul 29, 2020

jonathan-buttner Jul 29, 2020

jonathan-buttner commented Jul 29, 2020

kibanamachine commented Jul 29, 2020

[Security Solution][Resolver] Handle disabled process collection #73592

[Security Solution][Resolver] Handle disabled process collection #73592

Conversation

jonathan-buttner commented Jul 29, 2020 • edited Loading

elasticmachine commented Jul 29, 2020

elasticmachine commented Jul 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathan-buttner commented Jul 29, 2020

kibanamachine commented Jul 29, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

jonathan-buttner commented Jul 29, 2020 •

edited

Loading