From 39fe0aee76d0383258be8f68bed2865575ec20d2 Mon Sep 17 00:00:00 2001 From: Konrad Szwarc Date: Mon, 23 Sep 2024 14:02:16 +0200 Subject: [PATCH] [EDR Workflows] Set Agent Tamper Protection to false on policy unassignment (#193017) This PR fixes an issue where Agent Tamper Protection remained enabled even after a Defend policy was unassigned from the agent policy. There is no issue with removing the integration; it still causes agent tamper protections to be disabled. https://github.com/user-attachments/assets/ee105e60-3db2-4249-a33b-44a2f2f7aac9 --- .../server/services/package_policy.test.ts | 402 ++++++++++++++++++ .../fleet/server/services/package_policy.ts | 35 +- 2 files changed, 436 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/fleet/server/services/package_policy.test.ts b/x-pack/plugins/fleet/server/services/package_policy.test.ts index f0558aaa8fe265..ff15ae62cc7953 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.test.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import type { ElasticsearchClientMock } from '@kbn/core/server/mocks'; import { elasticsearchServiceMock, savedObjectsClientMock, @@ -34,6 +35,7 @@ import type { PackagePolicy, PostPackagePolicyPostCreateCallback, PostPackagePolicyDeleteCallback, + UpdatePackagePolicy, } from '../types'; import { createPackagePolicyMock } from '../../common/mocks'; @@ -1715,6 +1717,187 @@ describe('Package policy service', () => { savedObjectType: LEGACY_PACKAGE_POLICY_SAVED_OBJECT_TYPE, }); }); + + describe('remove protections', () => { + beforeEach(() => { + mockAgentPolicyService.bumpRevision.mockReset(); + }); + + const generateAttributes = (overrides: Record = {}) => ({ + name: 'endpoint-12', + description: '', + namespace: 'default', + enabled: true, + policy_ids: ['test'], + package: { + name: 'endpoint', + title: 'Elastic Endpoint', + version: '0.9.0', + }, + inputs: [], + ...overrides, + }); + + const generateSO = (overrides: Record = {}) => ({ + id: 'existing-package-policy', + type: 'ingest-package-policies', + references: [], + version: '1.0.0', + attributes: generateAttributes(overrides), + }); + + const testedPolicyIds = ['test-agent-policy-1', 'test-agent-policy-2', 'test-agent-policy-3']; + + const setupSOClientMocks = ( + savedObjectsClient: ReturnType, + initialPolicies: string[], + updatesPolicies: string[] + ) => { + savedObjectsClient.get.mockResolvedValueOnce( + generateSO({ name: 'test-package-policy', policy_ids: initialPolicies }) + ); + + savedObjectsClient.get.mockResolvedValueOnce( + generateSO({ name: 'test-package-policy-1', policy_ids: updatesPolicies }) + ); + + savedObjectsClient.get.mockResolvedValueOnce( + generateSO({ name: 'test-package-policy-1', policy_ids: updatesPolicies }) + ); + }; + + const callPackagePolicyServiceUpdate = async ( + savedObjectsClient: ReturnType, + elasticsearchClient: ElasticsearchClientMock, + policyIds: string[] + ) => { + await packagePolicyService.update( + savedObjectsClient, + elasticsearchClient, + generateSO().id, + generateAttributes({ + policy_ids: policyIds, + name: 'test-package-policy-1', + }) + ); + }; + + it('should not remove protections if policy_ids is not changed', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + setupSOClientMocks(savedObjectsClient, testedPolicyIds, testedPolicyIds); + + await callPackagePolicyServiceUpdate( + savedObjectsClient, + elasticsearchClient, + testedPolicyIds + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(testedPolicyIds.length); + Array.from({ length: testedPolicyIds.length }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: false }) + ); + }); + }); + + it('should remove protections if policy_ids is changed, only affected policies', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + const updatedPolicyIds = [...testedPolicyIds].splice(1, 2); + + setupSOClientMocks(savedObjectsClient, testedPolicyIds, updatedPolicyIds); + + await callPackagePolicyServiceUpdate( + savedObjectsClient, + elasticsearchClient, + updatedPolicyIds + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(testedPolicyIds.length); + Array.from({ length: testedPolicyIds.length }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: index === 1 }) + ); + }); + }); + + it('should remove protections from all agent policies if updated policy_ids is empty', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + setupSOClientMocks(savedObjectsClient, testedPolicyIds, []); + + await callPackagePolicyServiceUpdate(savedObjectsClient, elasticsearchClient, []); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(testedPolicyIds.length); + Array.from({ length: testedPolicyIds.length }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: true }) + ); + }); + }); + + it('should set protections to false on new policy assignment', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + const updatedPolicyIds = [...testedPolicyIds, 'test-agent-policy-4']; + + setupSOClientMocks(savedObjectsClient, testedPolicyIds, updatedPolicyIds); + + await callPackagePolicyServiceUpdate( + savedObjectsClient, + elasticsearchClient, + updatedPolicyIds + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(updatedPolicyIds.length); + Array.from({ length: testedPolicyIds.length }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: index === 4 }) // Only the last policy should have removeProtection set to true since it's new + ); + }); + }); + + it('should set protections to false on all new policy assignment', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + setupSOClientMocks(savedObjectsClient, [], testedPolicyIds); + + await callPackagePolicyServiceUpdate(savedObjectsClient, elasticsearchClient, []); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(testedPolicyIds.length); + Array.from({ length: testedPolicyIds.length }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: true }) + ); + }); + }); + }); }); describe('bulkUpdate', () => { @@ -2521,6 +2704,225 @@ describe('Package policy service', () => { }, ]); }); + + describe('remove protections', () => { + beforeEach(() => { + mockAgentPolicyService.bumpRevision.mockReset(); + }); + const generateAttributes = (overrides: Record = {}) => ({ + name: 'endpoint-12', + description: '', + namespace: 'default', + enabled: true, + policy_ids: ['test'], + package: { + name: 'endpoint', + title: 'Elastic Endpoint', + version: '0.9.0', + }, + inputs: [], + ...overrides, + }); + + const generateSO = (overrides: Record = {}) => ({ + id: 'existing-package-policy', + type: 'ingest-package-policies', + references: [], + version: '1.0.0', + attributes: generateAttributes(overrides), + ...(overrides.id ? ({ id: overrides.id } as { id: string }) : {}), + }); + + const packagePoliciesSO = [ + generateSO({ + name: 'test-package-policy', + policy_ids: ['test-agent-policy-1', 'test-agent-policy-2', 'test-agent-policy-3'], + id: 'asdb', + }), + generateSO({ + name: 'test-package-policy-1', + policy_ids: ['test-agent-policy-4', 'test-agent-policy-5', 'test-agent-policy-6'], + id: 'asdb1', + }), + ]; + const testedPackagePolicies = packagePoliciesSO.map((so) => so.attributes); + + const totalPolicyIds = packagePoliciesSO.reduce( + (count, policy) => count + policy.attributes.policy_ids.length, + 0 + ); + + const setupSOClientMocks = ( + savedObjectsClient: ReturnType, + overrideReturnedSOs?: typeof packagePoliciesSO + ) => { + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: overrideReturnedSOs || packagePoliciesSO, + }); + + savedObjectsClient.bulkUpdate.mockImplementation( + async ( + objs: Array<{ + type: string; + id: string; + attributes: any; + }> + ) => { + const newObjs = objs.map((obj) => ({ + id: 'test', + type: 'abcd', + references: [], + version: 'test', + attributes: obj.attributes, + })); + + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: newObjs, + }); + return { + saved_objects: newObjs, + }; + } + ); + }; + + const callPackagePolicyServiceBulkUpdate = async ( + savedObjectsClient: ReturnType, + elasticsearchClient: ElasticsearchClientMock, + packagePolicies: UpdatePackagePolicy[] + ) => { + await packagePolicyService.bulkUpdate( + savedObjectsClient, + elasticsearchClient, + packagePolicies, + { force: true } + ); + }; + + it('should not remove protections if policy_ids is not changed', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + + setupSOClientMocks(savedObjectsClient); + + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + await callPackagePolicyServiceBulkUpdate( + savedObjectsClient, + elasticsearchClient, + testedPackagePolicies + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(totalPolicyIds); + + Array.from({ length: totalPolicyIds }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: false }) + ); + }); + }); + + it('should remove protections if policy_ids is changed, only affected policies', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + + setupSOClientMocks(savedObjectsClient); + + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + const packagePoliciesWithIncompletePolicyIds = testedPackagePolicies.map((policy) => ({ + ...policy, + policy_ids: [...policy.policy_ids].splice(1, 2), + })); + + await callPackagePolicyServiceBulkUpdate( + savedObjectsClient, + elasticsearchClient, + packagePoliciesWithIncompletePolicyIds + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(totalPolicyIds); + + Array.from({ length: totalPolicyIds }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledWith( + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: index === 1 || index === 4 }) + ); + }); + }); + + it('should remove protections from all agent policies if updated policy_ids is empty', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + + setupSOClientMocks(savedObjectsClient); + + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + const packagePoliciesWithEmptyPolicyIds = testedPackagePolicies.map((policy) => ({ + ...policy, + policy_ids: [], + })); + + await callPackagePolicyServiceBulkUpdate( + savedObjectsClient, + elasticsearchClient, + packagePoliciesWithEmptyPolicyIds + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(totalPolicyIds); + + Array.from({ length: totalPolicyIds }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: true }) + ); + }); + }); + + it('should remove protections from all newly assigned policies', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + + setupSOClientMocks(savedObjectsClient, [ + generateSO({ + name: 'test-package-policy', + policy_ids: ['test-agent-policy-1'], + id: 'asdb', + }), + generateSO({ + name: 'test-package-policy-1', + policy_ids: [], + id: 'asdb1', + }), + ]); + + const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + await callPackagePolicyServiceBulkUpdate( + savedObjectsClient, + elasticsearchClient, + testedPackagePolicies + ); + + expect(mockAgentPolicyService.bumpRevision).toHaveBeenCalledTimes(totalPolicyIds); + + Array.from({ length: totalPolicyIds }, (_, index) => index + 1).forEach((index) => { + expect(mockAgentPolicyService.bumpRevision).toHaveBeenNthCalledWith( + index, + savedObjectsClient, + elasticsearchClient, + expect.stringContaining(`test-agent-policy-${index}`), + expect.objectContaining({ removeProtection: index !== 1 }) // First policy should not have protection removed since it was already assigned + ); + }); + }); + }); }); describe('delete', () => { diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index bda503257b4813..c457da64ead07b 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -1035,9 +1035,19 @@ class PackagePolicyClientImpl implements PackagePolicyClient { logger.debug(`Bumping revision of associated agent policies ${associatedPolicyIds}`); const bumpPromises = []; for (const policyId of associatedPolicyIds) { + // Check if the agent policy is in both old and updated package policies + const assignedInOldPolicy = oldPackagePolicy.policy_ids.includes(policyId); + const assignedInNewPolicy = newPolicy.policy_ids.includes(policyId); + + // Remove protection if policy is unassigned (in old but not in updated) or policy is assigned (in updated but not in old) + const removeProtection = + (assignedInOldPolicy && !assignedInNewPolicy) || + (!assignedInOldPolicy && assignedInNewPolicy); + bumpPromises.push( agentPolicyService.bumpRevision(soClient, esClient, policyId, { user: options?.user, + removeProtection, }) ); } @@ -1207,10 +1217,33 @@ class PackagePolicyClientImpl implements PackagePolicyClient { ...packagePolicyUpdates.flatMap((p) => p.policy_ids), ...oldPackagePolicies.flatMap((p) => p.policy_ids), ]); - logger.debug(`Bumping revision of associated agent policies ${associatedPolicyIds}`); + + const [endpointPackagePolicyUpdatesIds, endpointOldPackagePoliciesIds] = [ + packagePolicyUpdates, + oldPackagePolicies, + ].map( + (packagePolicies) => + new Set( + packagePolicies + .filter((p) => p.package?.name === 'endpoint') + .map((p) => p.policy_ids) + .flat() + ) + ); + const bumpPromise = pMap(associatedPolicyIds, async (agentPolicyId) => { + // Check if the agent policy is in both old and updated package policies + const assignedInOldPolicies = endpointOldPackagePoliciesIds.has(agentPolicyId); + const assignedInUpdatedPolicies = endpointPackagePolicyUpdatesIds.has(agentPolicyId); + + // Remove protection if policy is unassigned (in old but not in updated) or policy is assigned (in updated but not in old) + const removeProtection = + (assignedInOldPolicies && !assignedInUpdatedPolicies) || + (!assignedInOldPolicies && assignedInUpdatedPolicies); + await agentPolicyService.bumpRevision(soClient, esClient, agentPolicyId, { user: options?.user, + removeProtection, }); });