From 51f4c6399adde46da009aaa60565cf20ba051dd1 Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Mon, 16 Sep 2024 15:59:11 +0200 Subject: [PATCH 1/2] set Agent Tamper Protection to false on policy unassignment --- .../server/services/package_policy.test.ts | 318 ++++++++++++++++++ .../fleet/server/services/package_policy.ts | 24 +- 2 files changed, 341 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 f0558aaa8fe26..254d9baef05f5 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,141 @@ 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 }) + ); + }); + }); + }); }); describe('bulkUpdate', () => { @@ -2521,6 +2658,187 @@ 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 + ) => { + savedObjectsClient.bulkGet.mockResolvedValue({ + saved_objects: 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 }) + ); + }); + }); + }); }); 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 32058445d0745..1022e5e4e6ab9 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -1035,9 +1035,16 @@ class PackagePolicyClientImpl implements PackagePolicyClient { logger.debug(`Bumping revision of associated agent policies ${associatedPolicyIds}`); const bumpPromises = []; for (const policyId of associatedPolicyIds) { + // If the agent policy is no longer associated with the endpoint package policy, remove the protection + const removeProtection = + newPolicy.package?.name === 'endpoint' && + oldPackagePolicy.policy_ids.includes(policyId) && + !newPolicy.policy_ids.includes(policyId); + bumpPromises.push( agentPolicyService.bumpRevision(soClient, esClient, policyId, { user: options?.user, + removeProtection, }) ); } @@ -1207,10 +1214,25 @@ 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) => + packagePolicies + .filter((p) => p.package?.name === 'endpoint') + .map((p) => p.policy_ids) + .flat() + ); + const bumpPromise = pMap(associatedPolicyIds, async (agentPolicyId) => { + // If the agent policy is no longer associated with the endpoint package policy, remove the protection + const removeProtection = + endpointOldPackagePoliciesIds.includes(agentPolicyId) && + !endpointPackagePolicyUpdatesIds.includes(agentPolicyId); await agentPolicyService.bumpRevision(soClient, esClient, agentPolicyId, { user: options?.user, + removeProtection, }); }); From 7f5875658f048fde6c2cb985951347fa0d6a1384 Mon Sep 17 00:00:00 2001 From: "konrad.szwarc" Date: Fri, 20 Sep 2024 12:08:29 +0200 Subject: [PATCH 2/2] cr --- .../server/services/package_policy.test.ts | 88 ++++++++++++++++++- .../fleet/server/services/package_policy.ts | 35 +++++--- 2 files changed, 109 insertions(+), 14 deletions(-) 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 254d9baef05f5..ff15ae62cc795 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.test.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.test.ts @@ -1851,6 +1851,52 @@ describe('Package policy service', () => { ); }); }); + + 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 }) + ); + }); + }); }); }); @@ -2707,10 +2753,11 @@ describe('Package policy service', () => { ); const setupSOClientMocks = ( - savedObjectsClient: ReturnType + savedObjectsClient: ReturnType, + overrideReturnedSOs?: typeof packagePoliciesSO ) => { savedObjectsClient.bulkGet.mockResolvedValue({ - saved_objects: packagePoliciesSO, + saved_objects: overrideReturnedSOs || packagePoliciesSO, }); savedObjectsClient.bulkUpdate.mockImplementation( @@ -2838,6 +2885,43 @@ describe('Package policy service', () => { ); }); }); + + 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 + ); + }); + }); }); }); diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 0e1388408b9b1..c457da64ead07 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -1035,11 +1035,14 @@ class PackagePolicyClientImpl implements PackagePolicyClient { logger.debug(`Bumping revision of associated agent policies ${associatedPolicyIds}`); const bumpPromises = []; for (const policyId of associatedPolicyIds) { - // If the agent policy is no longer associated with the endpoint package policy, remove the protection + // 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 = - newPolicy.package?.name === 'endpoint' && - oldPackagePolicy.policy_ids.includes(policyId) && - !newPolicy.policy_ids.includes(policyId); + (assignedInOldPolicy && !assignedInNewPolicy) || + (!assignedInOldPolicy && assignedInNewPolicy); bumpPromises.push( agentPolicyService.bumpRevision(soClient, esClient, policyId, { @@ -1218,18 +1221,26 @@ class PackagePolicyClientImpl implements PackagePolicyClient { const [endpointPackagePolicyUpdatesIds, endpointOldPackagePoliciesIds] = [ packagePolicyUpdates, oldPackagePolicies, - ].map((packagePolicies) => - packagePolicies - .filter((p) => p.package?.name === 'endpoint') - .map((p) => p.policy_ids) - .flat() + ].map( + (packagePolicies) => + new Set( + packagePolicies + .filter((p) => p.package?.name === 'endpoint') + .map((p) => p.policy_ids) + .flat() + ) ); const bumpPromise = pMap(associatedPolicyIds, async (agentPolicyId) => { - // If the agent policy is no longer associated with the endpoint package policy, remove the protection + // 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 = - endpointOldPackagePoliciesIds.includes(agentPolicyId) && - !endpointPackagePolicyUpdatesIds.includes(agentPolicyId); + (assignedInOldPolicies && !assignedInUpdatedPolicies) || + (!assignedInOldPolicies && assignedInUpdatedPolicies); + await agentPolicyService.bumpRevision(soClient, esClient, agentPolicyId, { user: options?.user, removeProtection,