From a6582337e1c8b7f8f335694b92f791cb274dc519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20=C3=81brah=C3=A1m?= Date: Tue, 28 Nov 2023 15:11:24 +0100 Subject: [PATCH] [Defend Workflows][8.12 port] Unblock fleet setup when cannot decrypt uninstall tokens (#172058) ## Summary This PR is the `8.12` port of: - #171998 The original PR was opened to `8.11` to make it faster to include it in `8.12.2`. Now this PR is meant to port the changes to `main`, so: - we can build upon it, - and can easily backport any further changes to `8.11.x` > [!Important] > The changes cannot be tested on `main` because they are hidden by other behaviours (namely the retry logic for reading Message SIgning key) that weren't part of `8.11`. Those behaviours will be also adapted in follow up PRs. --- x-pack/plugins/fleet/server/mocks/index.ts | 2 + .../server/services/agent_policy.test.ts | 43 ++++++++-- .../fleet/server/services/agent_policy.ts | 15 ++++ .../uninstall_token_service/index.test.ts | 75 +++++++++++++++++ .../security/uninstall_token_service/index.ts | 80 +++++++++++-------- x-pack/plugins/fleet/server/services/setup.ts | 16 +++- 6 files changed, 189 insertions(+), 42 deletions(-) diff --git a/x-pack/plugins/fleet/server/mocks/index.ts b/x-pack/plugins/fleet/server/mocks/index.ts index 2716fd82b6811..ec8ada164623d 100644 --- a/x-pack/plugins/fleet/server/mocks/index.ts +++ b/x-pack/plugins/fleet/server/mocks/index.ts @@ -201,5 +201,7 @@ export function createUninstallTokenServiceMock(): UninstallTokenServiceInterfac generateTokensForPolicyIds: jest.fn(), generateTokensForAllPolicies: jest.fn(), encryptTokens: jest.fn(), + checkTokenValidityForAllPolicies: jest.fn(), + checkTokenValidityForPolicy: jest.fn(), }; } diff --git a/x-pack/plugins/fleet/server/services/agent_policy.test.ts b/x-pack/plugins/fleet/server/services/agent_policy.test.ts index 710ac46b94592..b6950ba672817 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.test.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.test.ts @@ -32,6 +32,7 @@ import { getFullAgentPolicy } from './agent_policies'; import * as outputsHelpers from './agent_policies/outputs_helpers'; import { auditLoggingService } from './audit_logging'; import { licenseService } from './license'; +import type { UninstallTokenServiceInterface } from './security/uninstall_token_service'; function getSavedObjectMock(agentPolicyAttributes: any) { const mock = savedObjectsClientMock.create(); @@ -182,13 +183,13 @@ describe('agent policy', () => { }); }); - it('should throw FleetUnauthorizedError if is_protected=true with insufficient license', () => { + it('should throw FleetUnauthorizedError if is_protected=true with insufficient license', async () => { jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(false); const soClient = getAgentPolicyCreateMock(); const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; - expect( + await expect( agentPolicyService.create(soClient, esClient, { name: 'test', namespace: 'default', @@ -199,13 +200,13 @@ describe('agent policy', () => { ); }); - it('should not throw FleetUnauthorizedError if is_protected=false with insufficient license', () => { + it('should not throw FleetUnauthorizedError if is_protected=false with insufficient license', async () => { jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(false); const soClient = getAgentPolicyCreateMock(); const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; - expect( + await expect( agentPolicyService.create(soClient, esClient, { name: 'test', namespace: 'default', @@ -619,7 +620,7 @@ describe('agent policy', () => { }); }); - it('should throw FleetUnauthorizedError if is_protected=true with insufficient license', () => { + it('should throw FleetUnauthorizedError if is_protected=true with insufficient license', async () => { jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(false); const soClient = getAgentPolicyCreateMock(); @@ -632,7 +633,7 @@ describe('agent policy', () => { references: [], }); - expect( + await expect( agentPolicyService.update(soClient, esClient, 'test-id', { name: 'test', namespace: 'default', @@ -643,7 +644,7 @@ describe('agent policy', () => { ); }); - it('should not throw FleetUnauthorizedError if is_protected=false with insufficient license', () => { + it('should not throw FleetUnauthorizedError if is_protected=false with insufficient license', async () => { jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(false); const soClient = getAgentPolicyCreateMock(); @@ -656,7 +657,7 @@ describe('agent policy', () => { references: [], }); - expect( + await expect( agentPolicyService.update(soClient, esClient, 'test-id', { name: 'test', namespace: 'default', @@ -665,6 +666,32 @@ describe('agent policy', () => { new FleetUnauthorizedError('Tamper protection requires Platinum license') ); }); + + it('should throw Error if is_protected=true with invalid uninstall token', async () => { + jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(true); + + mockedAppContextService.getUninstallTokenService.mockReturnValueOnce({ + checkTokenValidityForPolicy: jest.fn().mockRejectedValueOnce(new Error('reason')), + } as unknown as UninstallTokenServiceInterface); + + const soClient = getAgentPolicyCreateMock(); + const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + soClient.get.mockResolvedValue({ + attributes: {}, + id: 'test-id', + type: 'mocked', + references: [], + }); + + await expect( + agentPolicyService.update(soClient, esClient, 'test-id', { + name: 'test', + namespace: 'default', + is_protected: true, + }) + ).rejects.toThrowError(new Error('Cannot enable Agent Tamper Protection: reason')); + }); }); describe('deployPolicy', () => { diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index 8673bd6ad91a9..5e8c897d5611a 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -512,6 +512,7 @@ class AgentPolicyService { } this.checkTamperProtectionLicense(agentPolicy); + await this.checkForValidUninstallToken(agentPolicy, id); const logger = appContextService.getLogger(); @@ -1212,6 +1213,20 @@ class AgentPolicyService { throw new FleetUnauthorizedError('Tamper protection requires Platinum license'); } } + private async checkForValidUninstallToken( + agentPolicy: { is_protected?: boolean }, + policyId: string + ): Promise { + if (agentPolicy?.is_protected) { + const uninstallTokenService = appContextService.getUninstallTokenService(); + + try { + await uninstallTokenService?.checkTokenValidityForPolicy(policyId); + } catch (e) { + throw new Error(`Cannot enable Agent Tamper Protection: ${e.message}`); + } + } + } } export const agentPolicyService = new AgentPolicyService(); diff --git a/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.test.ts b/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.test.ts index 4cf657b7255c5..4b3be1de81632 100644 --- a/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.test.ts +++ b/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.test.ts @@ -499,5 +499,80 @@ describe('UninstallTokenService', () => { }); }); }); + + describe('check validity of tokens', () => { + const okaySO = getDefaultSO(canEncrypt); + + const errorWithDecryptionSO2 = { + ...getDefaultSO2(canEncrypt), + error: new Error('error reason'), + }; + const missingTokenSO2 = { + ...getDefaultSO2(canEncrypt), + attributes: { + ...getDefaultSO2(canEncrypt).attributes, + token: undefined, + token_plain: undefined, + }, + }; + + describe('checkTokenValidityForAllPolicies', () => { + it('resolves if all of the tokens are available', async () => { + mockCreatePointInTimeFinderAsInternalUser(); + + await expect( + uninstallTokenService.checkTokenValidityForAllPolicies() + ).resolves.not.toThrowError(); + }); + + it('rejects if any of the tokens is missing', async () => { + mockCreatePointInTimeFinderAsInternalUser([okaySO, missingTokenSO2]); + + await expect( + uninstallTokenService.checkTokenValidityForAllPolicies() + ).rejects.toThrowError( + 'Invalid uninstall token: Saved object is missing the `token` attribute.' + ); + }); + + it('rejects if token decryption gives error', async () => { + mockCreatePointInTimeFinderAsInternalUser([okaySO, errorWithDecryptionSO2]); + + await expect( + uninstallTokenService.checkTokenValidityForAllPolicies() + ).rejects.toThrowError('Error when reading Uninstall Token: error reason'); + }); + }); + + describe('checkTokenValidityForPolicy', () => { + it('resolves if token is available', async () => { + mockCreatePointInTimeFinderAsInternalUser(); + + await expect( + uninstallTokenService.checkTokenValidityForPolicy(okaySO.attributes.policy_id) + ).resolves.not.toThrowError(); + }); + + it('rejects if token is missing', async () => { + mockCreatePointInTimeFinderAsInternalUser([okaySO, missingTokenSO2]); + + await expect( + uninstallTokenService.checkTokenValidityForPolicy(missingTokenSO2.attributes.policy_id) + ).rejects.toThrowError( + 'Invalid uninstall token: Saved object is missing the `token` attribute.' + ); + }); + + it('rejects if token decryption gives error', async () => { + mockCreatePointInTimeFinderAsInternalUser([okaySO, errorWithDecryptionSO2]); + + await expect( + uninstallTokenService.checkTokenValidityForPolicy( + errorWithDecryptionSO2.attributes.policy_id + ) + ).rejects.toThrowError('Error when reading Uninstall Token: error reason'); + }); + }); + }); }); }); diff --git a/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts b/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts index 8309910be6f53..9e03e7869c584 100644 --- a/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts +++ b/x-pack/plugins/fleet/server/services/security/uninstall_token_service/index.ts @@ -109,7 +109,7 @@ export interface UninstallTokenServiceInterface { * @param force generate a new token even if one already exists * @returns hashedToken */ - generateTokenForPolicyId(policyId: string, force?: boolean): Promise; + generateTokenForPolicyId(policyId: string, force?: boolean): Promise; /** * Generate uninstall tokens for given policy ids @@ -119,7 +119,7 @@ export interface UninstallTokenServiceInterface { * @param force generate a new token even if one already exists * @returns Record */ - generateTokensForPolicyIds(policyIds: string[], force?: boolean): Promise>; + generateTokensForPolicyIds(policyIds: string[], force?: boolean): Promise; /** * Generate uninstall tokens all policies @@ -128,12 +128,26 @@ export interface UninstallTokenServiceInterface { * @param force generate a new token even if one already exists * @returns Record */ - generateTokensForAllPolicies(force?: boolean): Promise>; + generateTokensForAllPolicies(force?: boolean): Promise; /** * If encryption is available, checks for any plain text uninstall tokens and encrypts them */ encryptTokens(): Promise; + + /** + * Check whether the selected policy has a valid uninstall token. Rejects returning promise if not. + * + * @param policyId policy Id to check + */ + checkTokenValidityForPolicy(policyId: string): Promise; + + /** + * Check whether all policies have a valid uninstall token. Rejects returning promise if not. + * + * @param policyId policy Id to check + */ + checkTokenValidityForAllPolicies(): Promise; } export class UninstallTokenService implements UninstallTokenServiceInterface { @@ -210,7 +224,11 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { tokensFinder.close(); const uninstallTokens: UninstallToken[] = tokenObject.map( - ({ id: _id, attributes, created_at: createdAt }) => { + ({ id: _id, attributes, created_at: createdAt, error }) => { + if (error) { + throw new UninstallTokenError(`Error when reading Uninstall Token: ${error.message}`); + } + this.assertPolicyId(attributes); this.assertToken(attributes); this.assertCreatedAt(createdAt); @@ -304,32 +322,30 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { return this.getHashedTokensForPolicyIds(policyIds); } - public async generateTokenForPolicyId(policyId: string, force: boolean = false): Promise { - return (await this.generateTokensForPolicyIds([policyId], force))[policyId]; + public generateTokenForPolicyId(policyId: string, force: boolean = false): Promise { + return this.generateTokensForPolicyIds([policyId], force); } public async generateTokensForPolicyIds( policyIds: string[], force: boolean = false - ): Promise> { + ): Promise { const { agentTamperProtectionEnabled } = appContextService.getExperimentalFeatures(); if (!agentTamperProtectionEnabled || !policyIds.length) { - return {}; + return; } - const existingTokens = force - ? {} - : (await this.getDecryptedTokensForPolicyIds(policyIds)).reduce( - (acc, { policy_id: policyId, token }) => { - acc[policyId] = token; - return acc; - }, - {} as Record - ); + const existingTokens = new Set(); + + if (!force) { + (await this.getTokenObjectsByIncludeFilter(policyIds)).forEach((tokenObject) => { + existingTokens.add(tokenObject._source[UNINSTALL_TOKENS_SAVED_OBJECT_TYPE].policy_id); + }); + } const missingTokenPolicyIds = force ? policyIds - : policyIds.filter((policyId) => !existingTokens[policyId]); + : policyIds.filter((policyId) => !existingTokens.has(policyId)); const newTokensMap = missingTokenPolicyIds.reduce((acc, policyId) => { const token = this.generateToken(); @@ -338,7 +354,6 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { [policyId]: token, }; }, {} as Record); - await this.persistTokens(missingTokenPolicyIds, newTokensMap); if (force) { const config = appContextService.getConfig(); @@ -349,21 +364,9 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { await agentPolicyService.deployPolicies(this.soClient, policyIdsBatch) ); } - - const tokensMap = { - ...existingTokens, - ...newTokensMap, - }; - - return Object.entries(tokensMap).reduce((acc, [policyId, token]) => { - acc[policyId] = this.hashToken(token); - return acc; - }, {} as Record); } - public async generateTokensForAllPolicies( - force: boolean = false - ): Promise> { + public async generateTokensForAllPolicies(force: boolean = false): Promise { const policyIds = await this.getAllPolicyIds(); return this.generateTokensForPolicyIds(policyIds, force); } @@ -486,6 +489,15 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { return this._soClient; } + public async checkTokenValidityForPolicy(policyId: string): Promise { + await this.getDecryptedTokensForPolicyIds([policyId]); + } + + public async checkTokenValidityForAllPolicies(): Promise { + const policyIds = await this.getAllPolicyIds(); + await this.getDecryptedTokensForPolicyIds(policyIds); + } + private get isEncryptionAvailable(): boolean { return appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt ?? false; } @@ -498,7 +510,9 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { private assertToken(attributes: UninstallTokenSOAttributes | undefined) { if (!attributes?.token && !attributes?.token_plain) { - throw new UninstallTokenError('Uninstall Token is missing the token.'); + throw new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing the `token` attribute.' + ); } } diff --git a/x-pack/plugins/fleet/server/services/setup.ts b/x-pack/plugins/fleet/server/services/setup.ts index 178499011bc61..60ce6460d0ac2 100644 --- a/x-pack/plugins/fleet/server/services/setup.ts +++ b/x-pack/plugins/fleet/server/services/setup.ts @@ -6,6 +6,7 @@ */ import fs from 'fs/promises'; + import apm from 'elastic-apm-node'; import { compact } from 'lodash'; @@ -13,6 +14,8 @@ import pMap from 'p-map'; import type { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server'; import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common/constants'; +import type { UninstallTokenError } from '../../common/errors'; + import { AUTO_UPDATE_PACKAGES } from '../../common/constants'; import type { PreconfigurationError } from '../../common/constants'; import type { DefaultPackagesInstallationError } from '../../common/types'; @@ -54,7 +57,10 @@ import { cleanUpOldFileIndices } from './setup/clean_old_fleet_indices'; export interface SetupStatus { isInitialized: boolean; nonFatalErrors: Array< - PreconfigurationError | DefaultPackagesInstallationError | UpgradeManagedPackagePoliciesResult + | PreconfigurationError + | DefaultPackagesInstallationError + | UpgradeManagedPackagePoliciesResult + | { error: UninstallTokenError } >; } @@ -196,9 +202,17 @@ async function createSetupSideEffects( logger.debug('Checking for and encrypting plain text uninstall tokens'); await appContextService.getUninstallTokenService()?.encryptTokens(); } + + logger.debug('Checking validity of Uninstall Tokens'); + try { + await appContextService.getUninstallTokenService()?.checkTokenValidityForAllPolicies(); + } catch (error) { + nonFatalErrors.push({ error }); + } stepSpan?.end(); stepSpan = apm.startSpan('Upgrade agent policy schema', 'preconfiguration'); + logger.debug('Upgrade Agent policy schema version'); await upgradeAgentPolicySchemaVersion(soClient); stepSpan?.end();