From 17239f2f839746e41350e965876cfebcf3232040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20=C3=81brah=C3=A1m?= Date: Thu, 1 Feb 2024 11:58:33 +0100 Subject: [PATCH] [EDR Workflows][Fleet] Improve uninstall token validation in Fleet setup (#175679) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR is a proposal for improving Uninstall Token validation inside Fleet. Any feedback from @elastic/fleet team is very welcome 🙌 What it does: - moves Uninstall Token generation and validation from Fleet Setup to Fleet plugin start in order to not perform these steps every time `POST api/fleet/setup` is called - adds a summary to issues with uninstall tokens to Kibana logs e.g. ``` [2024-01-30T12:53:31.803+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,fb652173-7e07-47c1-8f42-e469d789d7ca": Unsupported state or unable to authenticate data [2024-01-30T12:53:31.885+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,2c34c587-f81c-4534-80e3-f45fb0d0c3f9": Unsupported state or unable to authenticate data [2024-01-30T12:53:31.886+01:00][ERROR][plugins.encryptedSavedObjects] Failed to decrypt attribute "token" of saved object "fleet-uninstall-tokens,e4d8cf22-0d8d-43c6-b21a-e11e7aea9932": Unsupported state or unable to authenticate data [2024-01-30T12:53:32.522+01:00][WARN ][plugins.fleet] Failed to decrypt 3 of 1130 Uninstall Token(s) ``` ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- x-pack/plugins/fleet/server/plugin.ts | 53 ++++++++ .../uninstall_token_service/index.test.ts | 96 +++++++++---- .../security/uninstall_token_service/index.ts | 128 ++++++++++++------ x-pack/plugins/fleet/server/services/setup.ts | 19 --- 4 files changed, 207 insertions(+), 89 deletions(-) diff --git a/x-pack/plugins/fleet/server/plugin.ts b/x-pack/plugins/fleet/server/plugin.ts index 8333d0257a8f0..93e3bab5529d4 100644 --- a/x-pack/plugins/fleet/server/plugin.ts +++ b/x-pack/plugins/fleet/server/plugin.ts @@ -583,6 +583,9 @@ export class FleetPlugin } ); + // initialize (generate/encrypt/validate) Uninstall Tokens asynchronously + this.initializeUninstallTokens(); + this.fleetStatus$.next({ level: ServiceStatusLevels.available, summary: 'Fleet is available', @@ -698,4 +701,54 @@ export class FleetPlugin return this.logger; } + + private async initializeUninstallTokens() { + try { + await this.generateUninstallTokens(); + } catch (error) { + appContextService + .getLogger() + .error('Error happened during uninstall token generation.', { error: { message: error } }); + } + + try { + await this.validateUninstallTokens(); + } catch (error) { + appContextService + .getLogger() + .error('Error happened during uninstall token validation.', { error: { message: error } }); + } + } + + private async generateUninstallTokens() { + const logger = appContextService.getLogger(); + + logger.debug('Generating Agent uninstall tokens'); + if (!appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) { + logger.warn( + 'xpack.encryptedSavedObjects.encryptionKey is not configured, agent uninstall tokens are being stored in plain text' + ); + } + await appContextService.getUninstallTokenService()?.generateTokensForAllPolicies(); + + if (appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) { + logger.debug('Checking for and encrypting plain text uninstall tokens'); + await appContextService.getUninstallTokenService()?.encryptTokens(); + } + } + + private async validateUninstallTokens() { + const logger = appContextService.getLogger(); + logger.debug('Validating uninstall tokens'); + + const unintallTokenValidationError = await appContextService + .getUninstallTokenService() + ?.checkTokenValidityForAllPolicies(); + + if (unintallTokenValidationError) { + logger.warn(unintallTokenValidationError.error.message); + } else { + logger.debug('Uninstall tokens validation successful.'); + } + } } 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 5dfd1e5c951f0..5cca694d844ab 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 @@ -31,6 +31,16 @@ import { agentPolicyService } from '../../agent_policy'; import { UninstallTokenService, type UninstallTokenServiceInterface } from '.'; +interface TokenSO { + id: string; + attributes: { + policy_id: string; + token?: string; + token_plain?: string; + }; + created_at: string; +} + describe('UninstallTokenService', () => { const now = new Date().toISOString(); const aDayAgo = new Date(Date.now() - 24 * 3600 * 1000).toISOString(); @@ -41,7 +51,7 @@ describe('UninstallTokenService', () => { let mockBuckets: any[] = []; let uninstallTokenService: UninstallTokenServiceInterface; - function getDefaultSO(encrypted: boolean = true) { + function getDefaultSO(encrypted: boolean = true): TokenSO { return encrypted ? { id: 'test-so-id', @@ -61,7 +71,7 @@ describe('UninstallTokenService', () => { }; } - function getDefaultSO2(encrypted: boolean = true) { + function getDefaultSO2(encrypted: boolean = true): TokenSO { return encrypted ? { id: 'test-so-id-two', @@ -81,6 +91,20 @@ describe('UninstallTokenService', () => { }; } + const decorateSOWithError = (so: TokenSO) => ({ + ...so, + error: new Error('error reason'), + }); + + const decorateSOWithMissingToken = (so: TokenSO) => ({ + ...so, + attributes: { + ...so.attributes, + token: undefined, + token_plain: undefined, + }, + }); + function getDefaultBuckets(encrypted: boolean = true) { const defaultSO = getDefaultSO(encrypted); const defaultSO2 = getDefaultSO2(encrypted); @@ -227,6 +251,26 @@ describe('UninstallTokenService', () => { } ); }); + + it('throws error if token is missing', async () => { + const so = decorateSOWithMissingToken(getDefaultSO(canEncrypt)); + mockCreatePointInTimeFinderAsInternalUser([so]); + + await expect(uninstallTokenService.getToken(so.id)).rejects.toThrowError( + new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing the token attribute.' + ) + ); + }); + + it("throws error if there's a depcryption error", async () => { + const so = decorateSOWithError(getDefaultSO2(canEncrypt)); + mockCreatePointInTimeFinderAsInternalUser([so]); + + await expect(uninstallTokenService.getToken(so.id)).rejects.toThrowError( + new UninstallTokenError("Error when reading Uninstall Token with id 'test-so-id-two'.") + ); + }); }); describe('getTokenMetadata', () => { @@ -507,18 +551,9 @@ 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, - }, - }; + const errorWithDecryptionSO1 = decorateSOWithError(getDefaultSO(canEncrypt)); + const errorWithDecryptionSO2 = decorateSOWithError(getDefaultSO2(canEncrypt)); + const missingTokenSO2 = decorateSOWithMissingToken(getDefaultSO2(canEncrypt)); describe('checkTokenValidityForAllPolicies', () => { it('returns null if all of the tokens are available', async () => { @@ -578,20 +613,31 @@ describe('UninstallTokenService', () => { uninstallTokenService.checkTokenValidityForAllPolicies() ).resolves.toStrictEqual({ error: new UninstallTokenError( - 'Invalid uninstall token: Saved object is missing the token attribute.' + 'Failed to validate Uninstall Tokens: 1 of 2 tokens are invalid' ), }); }); - it('returns error if token decryption gives error', async () => { + it('returns error if some of the tokens cannot be decrypted', async () => { mockCreatePointInTimeFinderAsInternalUser([okaySO, errorWithDecryptionSO2]); await expect( uninstallTokenService.checkTokenValidityForAllPolicies() ).resolves.toStrictEqual({ - error: new UninstallTokenError( - "Error when reading Uninstall Token with id 'test-so-id-two'." - ), + error: new UninstallTokenError('Failed to decrypt 1 of 2 Uninstall Token(s)'), + }); + }); + + it('returns error if none of the tokens can be decrypted', async () => { + mockCreatePointInTimeFinderAsInternalUser([ + errorWithDecryptionSO1, + errorWithDecryptionSO2, + ]); + + await expect( + uninstallTokenService.checkTokenValidityForAllPolicies() + ).resolves.toStrictEqual({ + error: new UninstallTokenError('Failed to decrypt 2 of 2 Uninstall Token(s)'), }); }); @@ -607,7 +653,7 @@ describe('UninstallTokenService', () => { }); describe('checkTokenValidityForPolicy', () => { - it('returns empty array if token is available', async () => { + it('returns null if token is available', async () => { mockCreatePointInTimeFinderAsInternalUser(); await expect( @@ -616,28 +662,26 @@ describe('UninstallTokenService', () => { }); it('returns error if token is missing', async () => { - mockCreatePointInTimeFinderAsInternalUser([okaySO, missingTokenSO2]); + mockCreatePointInTimeFinderAsInternalUser([missingTokenSO2]); await expect( uninstallTokenService.checkTokenValidityForPolicy(missingTokenSO2.attributes.policy_id) ).resolves.toStrictEqual({ error: new UninstallTokenError( - 'Invalid uninstall token: Saved object is missing the token attribute.' + 'Failed to validate Uninstall Tokens: 1 of 1 tokens are invalid' ), }); }); it('returns error if token decryption gives error', async () => { - mockCreatePointInTimeFinderAsInternalUser([okaySO, errorWithDecryptionSO2]); + mockCreatePointInTimeFinderAsInternalUser([errorWithDecryptionSO2]); await expect( uninstallTokenService.checkTokenValidityForPolicy( errorWithDecryptionSO2.attributes.policy_id ) ).resolves.toStrictEqual({ - error: new UninstallTokenError( - "Error when reading Uninstall Token with id 'test-so-id-two'." - ), + error: new UninstallTokenError('Failed to decrypt 1 of 1 Uninstall Token(s)'), }); }); 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 2215035684c67..3d43f280ba116 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 @@ -169,9 +169,9 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { public async getToken(id: string): Promise { const filter = `${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.id: "${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}:${id}"`; - const uninstallTokens = await this.getDecryptedTokens({ filter }); + const tokenObjects = await this.getDecryptedTokenObjects({ filter }); - return uninstallTokens.length === 1 ? uninstallTokens[0] : null; + return tokenObjects.length === 1 ? this.convertTokenObjectToToken(tokenObjects[0]) : null; } public async getTokenMetadata( @@ -201,6 +201,14 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { } private async getDecryptedTokensForPolicyIds(policyIds: string[]): Promise { + const tokenObjects = await this.getDecryptedTokenObjectsForPolicyIds(policyIds); + + return tokenObjects.map(this.convertTokenObjectToToken); + } + + private async getDecryptedTokenObjectsForPolicyIds( + policyIds: string[] + ): Promise>> { const tokenObjectHits = await this.getTokenObjectsByIncludeFilter(policyIds); if (tokenObjectHits.length === 0) { @@ -211,15 +219,33 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { ({ _id }) => `${UNINSTALL_TOKENS_SAVED_OBJECT_TYPE}.id: "${_id}"` ); - const uninstallTokenChunks: UninstallToken[][] = await asyncMap( - chunk(filterEntries, this.getUninstallTokenVerificationBatchSize()), - (entries) => { - const filter = entries.join(' or '); - return this.getDecryptedTokens({ filter }); + let tokenObjectChunks: Array>> = []; + + try { + tokenObjectChunks = await asyncMap( + chunk(filterEntries, this.getUninstallTokenVerificationBatchSize()), + async (entries) => { + const filter = entries.join(' or '); + return this.getDecryptedTokenObjects({ filter }); + } + ); + } catch (error) { + if (isResponseError(error) && error.message.includes('too_many_nested_clauses')) { + // `too_many_nested_clauses` is considered non-fatal + const errorMessage = + 'Failed to validate uninstall tokens: `too_many_nested_clauses` error received. ' + + 'Setting/decreasing the value of `xpack.fleet.setup.uninstallTokenVerificationBatchSize` in your kibana.yml should help. ' + + `Current value is ${this.getUninstallTokenVerificationBatchSize()}.`; + + appContextService.getLogger().warn(`${errorMessage}: '${error}'`); + + throw new UninstallTokenError(errorMessage); + } else { + throw error; } - ); + } - return uninstallTokenChunks.flat(); + return tokenObjectChunks.flat(); } private getUninstallTokenVerificationBatchSize = () => { @@ -232,9 +258,9 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { return config?.setup?.uninstallTokenVerificationBatchSize ?? 500; }; - private getDecryptedTokens = async ( + private async getDecryptedTokenObjects( options: Partial - ): Promise => { + ): Promise>> { const tokensFinder = await this.esoClient.createPointInTimeFinderDecryptedAsInternalUser( { @@ -243,34 +269,37 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { ...options, } ); - let tokenObject: Array> = []; + let tokenObjects: Array> = []; for await (const result of tokensFinder.find()) { - tokenObject = result.saved_objects; + tokenObjects = result.saved_objects; break; } tokensFinder.close(); - const uninstallTokens: UninstallToken[] = tokenObject.map( - ({ id: _id, attributes, created_at: createdAt, error }) => { - if (error) { - throw new UninstallTokenError(`Error when reading Uninstall Token with id '${_id}'.`); - } + return tokenObjects; + } - this.assertPolicyId(attributes); - this.assertToken(attributes); - this.assertCreatedAt(createdAt); + private convertTokenObjectToToken = ({ + id: _id, + attributes, + created_at: createdAt, + error, + }: SavedObjectsFindResult): UninstallToken => { + if (error) { + throw new UninstallTokenError(`Error when reading Uninstall Token with id '${_id}'.`); + } - return { - id: _id, - policy_id: attributes.policy_id, - token: attributes.token || attributes.token_plain, - created_at: createdAt, - }; - } - ); + this.assertPolicyId(attributes); + this.assertToken(attributes); + this.assertCreatedAt(createdAt); - return uninstallTokens; + return { + id: _id, + policy_id: attributes.policy_id, + token: attributes.token || attributes.token_plain, + created_at: createdAt, + }; }; private async getTokenObjectsByIncludeFilter( @@ -521,33 +550,44 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { public async checkTokenValidityForPolicy( policyId: string ): Promise { - return await this.checkTokenValidity([policyId]); + return await this.checkTokenValidityForPolicies([policyId]); } public async checkTokenValidityForAllPolicies(): Promise { const policyIds = await this.getAllPolicyIds(); - return await this.checkTokenValidity(policyIds); + return await this.checkTokenValidityForPolicies(policyIds); } - private async checkTokenValidity( + private async checkTokenValidityForPolicies( policyIds: string[] ): Promise { try { - await this.getDecryptedTokensForPolicyIds(policyIds); + const tokenObjects = await this.getDecryptedTokenObjectsForPolicyIds(policyIds); + + const numberOfDecryptionErrors = tokenObjects.filter(({ error }) => error).length; + if (numberOfDecryptionErrors > 0) { + return { + error: new UninstallTokenError( + `Failed to decrypt ${numberOfDecryptionErrors} of ${tokenObjects.length} Uninstall Token(s)` + ), + }; + } + + const numberOfTokensWithMissingData = tokenObjects.filter( + ({ attributes, created_at: createdAt }) => + !createdAt || !attributes.policy_id || (!attributes.token && !attributes.token_plain) + ).length; + if (numberOfTokensWithMissingData > 0) { + return { + error: new UninstallTokenError( + `Failed to validate Uninstall Tokens: ${numberOfTokensWithMissingData} of ${tokenObjects.length} tokens are invalid` + ), + }; + } } catch (error) { if (error instanceof UninstallTokenError) { // known errors are considered non-fatal return { error }; - } else if (isResponseError(error) && error.message.includes('too_many_nested_clauses')) { - // `too_many_nested_clauses` is considered non-fatal - const errorMessage = - 'Failed to validate uninstall tokens: `too_many_nested_clauses` error received. ' + - 'Setting/decreasing the value of `xpack.fleet.setup.uninstallTokenVerificationBatchSize` in your kibana.yml should help. ' + - `Current value is ${this.getUninstallTokenVerificationBatchSize()}.`; - - appContextService.getLogger().warn(`${errorMessage}: '${error}'`); - - return { error: new UninstallTokenError(errorMessage) }; } else { const errorMessage = 'Unknown error happened while checking Uninstall Tokens validity'; appContextService.getLogger().error(`${errorMessage}: '${error}'`); diff --git a/x-pack/plugins/fleet/server/services/setup.ts b/x-pack/plugins/fleet/server/services/setup.ts index 8d343e56b1035..af2c19c42c70b 100644 --- a/x-pack/plugins/fleet/server/services/setup.ts +++ b/x-pack/plugins/fleet/server/services/setup.ts @@ -197,24 +197,6 @@ async function createSetupSideEffects( throw error; } } - - logger.debug('Generating Agent uninstall tokens'); - if (!appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) { - logger.warn( - 'xpack.encryptedSavedObjects.encryptionKey is not configured, agent uninstall tokens are being stored in plain text' - ); - } - await appContextService.getUninstallTokenService()?.generateTokensForAllPolicies(); - - if (appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) { - logger.debug('Checking for and encrypting plain text uninstall tokens'); - await appContextService.getUninstallTokenService()?.encryptTokens(); - } - - logger.debug('Checking validity of Uninstall Tokens'); - const uninstallTokenError = await appContextService - .getUninstallTokenService() - ?.checkTokenValidityForAllPolicies(); stepSpan?.end(); stepSpan = apm.startSpan('Upgrade agent policy schema', 'preconfiguration'); @@ -234,7 +216,6 @@ async function createSetupSideEffects( ...preconfiguredPackagesNonFatalErrors, ...packagePolicyUpgradeErrors, ...(messageSigningServiceNonFatalError ? [messageSigningServiceNonFatalError] : []), - ...(uninstallTokenError ? [uninstallTokenError] : []), ]; if (nonFatalErrors.length > 0) {