From 5f53b649c60c481809994ad866f4c65d2e03d9fe Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 19 Jan 2021 17:36:05 -0500 Subject: [PATCH] [Security Solution][Endpoint Exceptions] - Fix bug where endpoint exceptions list not created when expected (#88232) ## Summary Addresses issue issue 87110 **Issue** When prepackaged rules were created during the Endpoint Security enrollment flow, the endpoint exceptions list was failing to be created. As a result, when a user navigated to the `Endpoint Security` rule to add an exception it would display errors and not allow a user to add exceptions. --- .../exception_list_client.mock.ts | 1 + .../endpoint/endpoint_app_context_services.ts | 5 +- .../endpoint/ingest_integration.test.ts | 41 +++++++++++++++-- .../server/endpoint/ingest_integration.ts | 7 ++- .../server/endpoint/mocks.ts | 2 + .../rules/add_prepackaged_rules_route.test.ts | 46 ++++++++++++++++++- .../rules/add_prepackaged_rules_route.ts | 11 ++++- .../security_solution/server/plugin.ts | 1 + 8 files changed, 104 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/lists/server/services/exception_lists/exception_list_client.mock.ts b/x-pack/plugins/lists/server/services/exception_lists/exception_list_client.mock.ts index 4354c735747b..ae1205743373 100644 --- a/x-pack/plugins/lists/server/services/exception_lists/exception_list_client.mock.ts +++ b/x-pack/plugins/lists/server/services/exception_lists/exception_list_client.mock.ts @@ -28,6 +28,7 @@ export class ExceptionListClientMock extends ExceptionListClient { public findExceptionListItem = jest.fn().mockResolvedValue(getFoundExceptionListItemSchemaMock()); public findExceptionList = jest.fn().mockResolvedValue(getFoundExceptionListSchemaMock()); public createTrustedAppsList = jest.fn().mockResolvedValue(getTrustedAppsListSchemaMock()); + public createEndpointList = jest.fn().mockResolvedValue(getExceptionListSchemaMock()); } export const getExceptionListClientMock = (): ExceptionListClient => { diff --git a/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts b/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts index 69d986707adb..ec3d35cbb658 100644 --- a/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts +++ b/x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.ts @@ -9,6 +9,7 @@ import { SavedObjectsServiceStart, SavedObjectsClientContract, } from 'src/core/server'; +import { ExceptionListClient } from '../../../lists/server'; import { SecurityPluginSetup } from '../../../security/server'; import { AgentService, @@ -90,6 +91,7 @@ export type EndpointAppContextServiceStartContract = Partial< registerIngestCallback?: FleetStartContract['registerExternalCallback']; savedObjectsStart: SavedObjectsServiceStart; licenseService: LicenseService; + exceptionListsClient: ExceptionListClient | undefined; }; /** @@ -121,7 +123,8 @@ export class EndpointAppContextService { dependencies.appClientFactory, dependencies.config.maxTimelineImportExportSize, dependencies.security, - dependencies.alerts + dependencies.alerts, + dependencies.exceptionListsClient ) ); diff --git a/x-pack/plugins/security_solution/server/endpoint/ingest_integration.test.ts b/x-pack/plugins/security_solution/server/endpoint/ingest_integration.test.ts index c0f95b5bce61..d2ce0908b91f 100644 --- a/x-pack/plugins/security_solution/server/endpoint/ingest_integration.test.ts +++ b/x-pack/plugins/security_solution/server/endpoint/ingest_integration.test.ts @@ -25,11 +25,14 @@ import { Subject } from 'rxjs'; import { ILicense } from '../../../licensing/common/types'; import { EndpointDocGenerator } from '../../common/endpoint/generate_data'; import { ProtectionModes } from '../../common/endpoint/types'; +import { getExceptionListClientMock } from '../../../lists/server/services/exception_lists/exception_list_client.mock'; +import { ExceptionListClient } from '../../../lists/server'; describe('ingest_integration tests ', () => { let endpointAppContextMock: EndpointAppContextServiceStartContract; let req: KibanaRequest; let ctx: RequestHandlerContext; + const exceptionListClient: ExceptionListClient = getExceptionListClientMock(); const maxTimelineImportExportSize = createMockConfig().maxTimelineImportExportSize; let licenseEmitter: Subject; let licenseService: LicenseService; @@ -63,7 +66,8 @@ describe('ingest_integration tests ', () => { endpointAppContextMock.appClientFactory, maxTimelineImportExportSize, endpointAppContextMock.security, - endpointAppContextMock.alerts + endpointAppContextMock.alerts, + exceptionListClient ); const policyConfig = createNewPackagePolicyMock(); // policy config without manifest const newPolicyConfig = await callback(policyConfig, ctx, req); // policy config WITH manifest @@ -140,7 +144,8 @@ describe('ingest_integration tests ', () => { endpointAppContextMock.appClientFactory, maxTimelineImportExportSize, endpointAppContextMock.security, - endpointAppContextMock.alerts + endpointAppContextMock.alerts, + exceptionListClient ); const policyConfig = createNewPackagePolicyMock(); const newPolicyConfig = await callback(policyConfig, ctx, req); @@ -167,7 +172,8 @@ describe('ingest_integration tests ', () => { endpointAppContextMock.appClientFactory, maxTimelineImportExportSize, endpointAppContextMock.security, - endpointAppContextMock.alerts + endpointAppContextMock.alerts, + exceptionListClient ); const policyConfig = createNewPackagePolicyMock(); const newPolicyConfig = await callback(policyConfig, ctx, req); @@ -188,7 +194,8 @@ describe('ingest_integration tests ', () => { endpointAppContextMock.appClientFactory, maxTimelineImportExportSize, endpointAppContextMock.security, - endpointAppContextMock.alerts + endpointAppContextMock.alerts, + exceptionListClient ); const policyConfig = createNewPackagePolicyMock(); const newPolicyConfig = await callback(policyConfig, ctx, req); @@ -199,6 +206,32 @@ describe('ingest_integration tests ', () => { lastComputed!.toEndpointFormat() ); }); + + test('policy creation succeeds even if endpoint exception list creation fails', async () => { + const mockError = new Error('error creating endpoint list'); + const logger = loggingSystemMock.create().get('ingest_integration.test'); + const manifestManager = getManifestManagerMock(); + const lastComputed = await manifestManager.getLastComputedManifest(); + exceptionListClient.createEndpointList = jest.fn().mockRejectedValue(mockError); + const callback = getPackagePolicyCreateCallback( + logger, + manifestManager, + endpointAppContextMock.appClientFactory, + maxTimelineImportExportSize, + endpointAppContextMock.security, + endpointAppContextMock.alerts, + exceptionListClient + ); + const policyConfig = createNewPackagePolicyMock(); + const newPolicyConfig = await callback(policyConfig, ctx, req); + + expect(exceptionListClient.createEndpointList).toHaveBeenCalled(); + expect(newPolicyConfig.inputs[0]!.type).toEqual('endpoint'); + expect(newPolicyConfig.inputs[0]!.config!.policy.value).toEqual(policyConfigFactory()); + expect(newPolicyConfig.inputs[0]!.config!.artifact_manifest.value).toEqual( + lastComputed!.toEndpointFormat() + ); + }); }); describe('when the license is below platinum', () => { beforeEach(() => { diff --git a/x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts b/x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts index 0143e34de31d..194d93df4b43 100644 --- a/x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts +++ b/x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { ExceptionListClient } from '../../../lists/server'; import { PluginStartContract as AlertsStartContract } from '../../../alerts/server'; import { SecurityPluginSetup } from '../../../security/server'; import { ExternalCallback } from '../../../fleet/server'; @@ -84,7 +85,8 @@ export const getPackagePolicyCreateCallback = ( appClientFactory: AppClientFactory, maxTimelineImportExportSize: number, securitySetup: SecurityPluginSetup, - alerts: AlertsStartContract + alerts: AlertsStartContract, + exceptionsClient: ExceptionListClient | undefined ): ExternalCallback[1] => { const handlePackagePolicyCreate = async ( newPackagePolicy: NewPackagePolicy, @@ -119,7 +121,8 @@ export const getPackagePolicyCreateCallback = ( appClient, alerts.getAlertsClientWithRequest(request), frameworkRequest, - maxTimelineImportExportSize + maxTimelineImportExportSize, + exceptionsClient ); } catch (err) { logger.error( diff --git a/x-pack/plugins/security_solution/server/endpoint/mocks.ts b/x-pack/plugins/security_solution/server/endpoint/mocks.ts index 5430e46b810c..2161ccf0eb78 100644 --- a/x-pack/plugins/security_solution/server/endpoint/mocks.ts +++ b/x-pack/plugins/security_solution/server/endpoint/mocks.ts @@ -6,6 +6,7 @@ import { ILegacyScopedClusterClient, SavedObjectsClientContract } from 'kibana/server'; import { loggingSystemMock, savedObjectsServiceMock } from 'src/core/server/mocks'; +import { listMock } from '../../../lists/server/mocks'; import { securityMock } from '../../../security/server/mocks'; import { alertsMock } from '../../../alerts/server/mocks'; import { xpackMocks } from '../../../../mocks'; @@ -79,6 +80,7 @@ export const createMockEndpointAppContextServiceStartContract = (): jest.Mocked< ReturnType, Parameters >(), + exceptionListsClient: listMock.getExceptionListClient(), }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts index 3ce8d08a57ac..3dc25583579c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts @@ -15,7 +15,11 @@ import { requestContextMock, serverMock, createMockConfig, mockGetCurrentUser } import { AddPrepackagedRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema'; import { SecurityPluginSetup } from '../../../../../../security/server'; import { installPrepackagedTimelines } from '../../../timeline/routes/utils/install_prepacked_timelines'; -import { addPrepackedRulesRoute } from './add_prepackaged_rules_route'; +import { addPrepackedRulesRoute, createPrepackagedRules } from './add_prepackaged_rules_route'; +import { listMock } from '../../../../../../lists/server/mocks'; +import { siemMock } from '../../../../mocks'; +import { FrameworkRequest } from '../../../framework'; +import { ExceptionListClient } from '../../../../../../lists/server'; jest.mock('../../rules/get_prepackaged_rules', () => { return { @@ -66,9 +70,11 @@ jest.mock('../../../timeline/routes/utils/install_prepacked_timelines', () => { }); describe('add_prepackaged_rules_route', () => { + const siemMockClient = siemMock.createClient(); let server: ReturnType; let { clients, context } = requestContextMock.createTools(); let securitySetup: SecurityPluginSetup; + let mockExceptionsClient: ExceptionListClient; beforeEach(() => { server = serverMock.create(); @@ -80,6 +86,8 @@ describe('add_prepackaged_rules_route', () => { authz: {}, } as unknown) as SecurityPluginSetup; + mockExceptionsClient = listMock.getExceptionListClient(); + clients.clusterClient.callAsCurrentUser.mockResolvedValue(getNonEmptyIndex()); clients.alertsClient.find.mockResolvedValue(getFindResultWithSingleHit()); @@ -271,4 +279,40 @@ describe('add_prepackaged_rules_route', () => { timelines_updated: 0, }); }); + + describe('createPrepackagedRules', () => { + test('uses exception lists client from context when available', async () => { + context.lists = { + getExceptionListClient: jest.fn(), + getListClient: jest.fn(), + }; + + await createPrepackagedRules( + context, + siemMockClient, + clients.alertsClient, + {} as FrameworkRequest, + 1200, + mockExceptionsClient + ); + + expect(mockExceptionsClient.createEndpointList).not.toHaveBeenCalled(); + expect(context.lists?.getExceptionListClient).toHaveBeenCalled(); + }); + + test('uses passed in exceptions list client when lists client not available in context', async () => { + const { lists, ...myContext } = context; + + await createPrepackagedRules( + myContext, + siemMockClient, + clients.alertsClient, + {} as FrameworkRequest, + 1200, + mockExceptionsClient + ); + + expect(mockExceptionsClient.createEndpointList).toHaveBeenCalled(); + }); + }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts index a3b378a6ef04..210ec87ade2d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts @@ -32,6 +32,8 @@ import { transformError, buildSiemResponse } from '../utils'; import { AlertsClient } from '../../../../../../alerts/server'; import { FrameworkRequest } from '../../../framework'; +import { ExceptionListClient } from '../../../../../../lists/server'; + export const addPrepackedRulesRoute = ( router: IRouter, config: ConfigType, @@ -89,17 +91,22 @@ export const createPrepackagedRules = async ( siemClient: AppClient, alertsClient: AlertsClient, frameworkRequest: FrameworkRequest, - maxTimelineImportExportSize: number + maxTimelineImportExportSize: number, + exceptionsClient?: ExceptionListClient ): Promise => { const clusterClient = context.core.elasticsearch.legacy.client; const savedObjectsClient = context.core.savedObjects.client; + const exceptionsListClient = + context.lists != null ? context.lists.getExceptionListClient() : exceptionsClient; if (!siemClient || !alertsClient) { throw new PrepackagedRulesError('', 404); } // This will create the endpoint list if it does not exist yet - await context.lists?.getExceptionListClient().createEndpointList(); + if (exceptionsListClient != null) { + await exceptionsListClient.createEndpointList(); + } const rulesFromFileSystem = getPrepackagedRules(); const prepackagedRules = await getExistingPrepackagedRules({ alertsClient }); diff --git a/x-pack/plugins/security_solution/server/plugin.ts b/x-pack/plugins/security_solution/server/plugin.ts index bb71041b4844..d51346ee9645 100644 --- a/x-pack/plugins/security_solution/server/plugin.ts +++ b/x-pack/plugins/security_solution/server/plugin.ts @@ -374,6 +374,7 @@ export class Plugin implements IPlugin