[Security Solution] Adds a version and OS check for Host Isolation #103026

kevinlog · 2021-06-23T00:02:17Z

Summary

This PR restricts Host Isolation access in the UI based on Agent version and OS.

We need to drop Linux support for the first release. In addition, we should disallow Host Isolation actions if the Agent/Endpoint is under 7.14.0.

I have the functions set up so that we should also be able to restrict based on this via the API

7.14.0 Endpoint with Windows:

Under 7.14.0 Endpoint:

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2021-06-23T14:04:41Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

kevinlog · 2021-06-23T17:34:35Z

@elasticmachine merge upstream

…restrict-os-and-version-for-host-isolation

kevinlog · 2021-06-25T16:26:17Z

x-pack/plugins/security_solution/common/endpoint/data_generators/base_data_generator.ts

@@ -102,7 +102,7 @@ export class BaseDataGenerator<GeneratedDoc extends {} = {}> {
  }

  protected randomVersion(): string {
-    return [6, ...this.randomNGenerator(10, 2)].map((x) => x.toString()).join('.');
+    return [7, ...this.randomNGenerator(20, 2)].map((x) => x.toString()).join('.');


Make the version generator possible to be 7.14.0+ to ensure fake docs will pass the version check.

kevinlog · 2021-06-25T16:27:37Z

x-pack/plugins/security_solution/common/endpoint/generate_data.ts

@@ -51,7 +51,7 @@ export const ANCESTRY_LIMIT: number = 2;

 const Windows: OSFields[] = [
  {
-    name: 'windows 10.0',
+    name: 'Windows',


Update the name field for OS in the generator to reflect reality from the Endpoint: https://github.com/elastic/endpoint-dev/blob/d233f42dde60a05fbfbd194695236c0c09341aa7/Elastic/Libraries/ECS/Lib/Constants.cpp#L192-L201

kevinlog · 2021-06-27T19:35:44Z

@elasticmachine merge upstream

…ation

academo · 2021-06-29T09:34:13Z

...ity_solution/public/management/pages/endpoint_hosts/view/hooks/use_endpoint_action_items.tsx

@@ -62,7 +67,7 @@ export const useEndpointActionItems = (

      const isolationActions = [];

-      if (isIsolated) {
+      if (isIsolated && isolationSupported) {


What are the (rather odd) chances that someone "isolated" an unsupported host via other means and now they can't release it through the UI?

Is this check necessary? if a host can't be isolated, certainly they should never reach the first part of the condition

In this case, if Isolation isn't supported, the Endpoint would never be able to actually isolate, even if we sent it an action. The Endpoint would ignore it because it doesn't recognize the action request. This will be true for old Endpoints and for Linux Endpoints in 7.14.

I'm thinking that regardless if it is supported or not, if we know the host is isolated, then we should allow unisolate. Hopefully this will never happen, but to be safe, I would revert this change here

academo · 2021-06-29T09:35:53Z

x-pack/plugins/security_solution/public/timelines/components/side_panel/event_details/index.tsx

-          <EuiSpacer size="l" />
-        </EuiFlyoutFooter>
-      )}
+      {isIsolationAllowed &&


I have a concern about hiding the TakeActionDropdown in its entirety. Are we planning to use this dropdown to add more actions later other than isolate?

@academo we will add new actions in the future, but currently there is just the one. We would revisit this check when the time comes

academo · 2021-06-29T09:37:43Z

x-pack/plugins/security_solution/common/endpoint/service/host_isolation/utils.test.ts

+
+import { isVersionSupported, isOsSupported, isIsolationSupported } from './utils';
+
+describe('Host Isolation utils isVersionSupported', () => {


not necessary to take action:

Jest support table tests which makes these kind of repetitive tests easier to read and write.

https://jestjs.io/docs/api#testeachtablename-fn-timeout

Example:

test.each` a | b | expected ${'8.14.9'} | ${'7.13.0'} | ${true} ${'8.14.9'} | ${'9.14.0'} | ${false} `('should validate that version $a is equal($expected) to $b', ({ a, b, expected }) => { expect( isVersionSupported({ currentVersion: a, minVersionRequired: b, }) ).toEqual(expected); });

thanks for the tip, I didn't know that! I've refactored my tests to reflect it.

paul-tavares

Left some comments. Let me know if you have any questions

paul-tavares · 2021-06-29T13:15:16Z

x-pack/plugins/security_solution/common/endpoint/service/host_isolation/utils.ts

+  minVersionRequired: string;
+}) => {
+  const parsedCurrentVersion = currentVersion.includes('-SNAPSHOT')
+    ? currentVersion.substring(0, currentVersion.length - 9)


Maybe ok for now, but semver can actually have other suffixes like -beta, -alpha, -build, etc. (reference).
My suggestion here is to instead take everything before the dash:

Suggested change

? currentVersion.substring(0, currentVersion.length - 9)

? currentVersion.substring(0, currentVersion.indexOf('-'))

Another alternative - use the semver package that is already available in kibana's node_modules

paul-tavares · 2021-06-29T13:17:37Z

...ity_solution/public/management/pages/endpoint_hosts/view/hooks/use_endpoint_action_items.tsx

@@ -62,7 +67,7 @@ export const useEndpointActionItems = (

      const isolationActions = [];

-      if (isIsolated) {
+      if (isIsolated && isolationSupported) {


I'm thinking that regardless if it is supported or not, if we know the host is isolated, then we should allow unisolate. Hopefully this will never happen, but to be safe, I would revert this change here

paul-tavares

🚢
👍

…restrict-os-and-version-for-host-isolation

kibanamachine · 2021-06-29T18:40:46Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2186	2187	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	6.3MB	6.3MB	+1.7KB

History

💚 Build #135101 succeeded a240ddb
💚 Build #134423 succeeded bdcda87
💚 Build #134362 succeeded af57cc1
💚 Build #134358 succeeded bf5d968
💚 Build #133701 succeeded 632a5f3

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

…lastic#103026)

kibanamachine · 2021-06-29T23:48:42Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…103026) (#103796) Co-authored-by: Kevin Logan <56395104+kevinlog@users.noreply.github.com>

kevinlog added 3 commits June 22, 2021 20:00

add initial log and tests

961d930

fix tests, adjust lookup

7c425fe

remove todos

fc8d3ac

kevinlog added v7.14.0 v8.0.0 release_note:feature Makes this part of the condensed release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Jun 23, 2021

kevinlog marked this pull request as ready for review June 23, 2021 14:04

kevinlog requested a review from a team as a code owner June 23, 2021 14:04

Merge branch 'master' of https://github.com/elastic/kibana into task/…

bf5d968

…restrict-os-and-version-for-host-isolation

kevinlog force-pushed the task/restrict-os-and-version-for-host-isolation branch from 632a5f3 to bf5d968 Compare June 25, 2021 15:41

update the generator so that fake docs will have Isoaltion enabled

af57cc1

kevinlog commented Jun 25, 2021

View reviewed changes

Merge branch 'master' into task/restrict-os-and-version-for-host-isol…

bdcda87

…ation

academo reviewed Jun 29, 2021

View reviewed changes

paul-tavares reviewed Jun 29, 2021

View reviewed changes

kevinlog added 2 commits June 29, 2021 09:34

refactor tests

4b2f3c6

pr comments

a240ddb

paul-tavares approved these changes Jun 29, 2021

View reviewed changes

pzl approved these changes Jun 29, 2021

View reviewed changes

academo approved these changes Jun 29, 2021

View reviewed changes

Merge branch 'master' of https://github.com/elastic/kibana into task/…

2d22901

…restrict-os-and-version-for-host-isolation

kevinlog added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 29, 2021

kevinlog merged commit 1ff2407 into elastic:master Jun 29, 2021

kevinlog deleted the task/restrict-os-and-version-for-host-isolation branch June 29, 2021 23:46

kibanamachine mentioned this pull request Jun 29, 2021

[7.x] [Security Solution] Adds a version and OS check for Host Isolation (#103026) #103796

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 29, 2021

[Security Solution] Adds a version and OS check for Host Isolation (e…

61ffd41

…lastic#103026)

kibanamachine added a commit that referenced this pull request Jun 30, 2021

[Security Solution] Adds a version and OS check for Host Isolation (#…

090fd8b

…103026) (#103796) Co-authored-by: Kevin Logan <56395104+kevinlog@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Adds a version and OS check for Host Isolation #103026

[Security Solution] Adds a version and OS check for Host Isolation #103026

kevinlog commented Jun 23, 2021 •

edited

Loading

elasticmachine commented Jun 23, 2021

kevinlog commented Jun 23, 2021

kevinlog Jun 25, 2021

kevinlog Jun 25, 2021

kevinlog commented Jun 27, 2021

academo Jun 29, 2021

kevinlog Jun 29, 2021

paul-tavares Jun 29, 2021

academo Jun 29, 2021

kevinlog Jun 29, 2021

academo Jun 29, 2021

academo Jun 29, 2021

kevinlog Jun 29, 2021

paul-tavares left a comment

paul-tavares Jun 29, 2021

kevinlog Jun 29, 2021

paul-tavares Jun 29, 2021

paul-tavares left a comment

kibanamachine commented Jun 29, 2021

kibanamachine commented Jun 29, 2021


		import { isVersionSupported, isOsSupported, isIsolationSupported } from './utils';

		describe('Host Isolation utils isVersionSupported', () => {

	? currentVersion.substring(0, currentVersion.length - 9)
	? currentVersion.substring(0, currentVersion.indexOf('-'))

[Security Solution] Adds a version and OS check for Host Isolation #103026

[Security Solution] Adds a version and OS check for Host Isolation #103026

Conversation

kevinlog commented Jun 23, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Jun 23, 2021

kevinlog commented Jun 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevinlog commented Jun 27, 2021

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

paul-tavares left a 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

paul-tavares left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Jun 29, 2021

💚 Backport successful

kevinlog commented Jun 23, 2021 •

edited

Loading