From abac134908338d8a16aa26bd7c9bd1197eb62e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20=C3=81brah=C3=A1m?= Date: Fri, 1 Dec 2023 12:49:02 +0100 Subject: [PATCH] [Defend Workflows] Handling message signing decryption fail during Fleet setup (#172072) ## Summary When encryption key is rotated improperly, Message Signing key retrieval is retried infinitely, and because this is done in Fleet setup, none of the Fleet pages are loaded when the user tries to visit any of them. This PR re-configures the retry logic so it now attempts significantly less times than infinite. Also, it changes the errors to non-fatal from the Fleet setup point of view, similarly to these PRs: - #171998 - #172058 ## Reproducing the encryption key issue - setup a Kibana, add at least one policy (probably Fleet policy is enough) - modify (or add a new) encryption key in your `kibana.dev.yml`: ```yml xpack.encryptedSavedObjects.encryptionKey: "some-random-encryption-key-min-32-bytes" ``` ## Screenshots After ~15 sec of loading spinner, this is what the user sees: ![image](https://github.com/elastic/kibana/assets/39014407/2a29d0d9-4975-46b5-b662-bfbb6e888b0f) ### 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 --- .../server/services/agent_policy.test.ts | 4 +- .../fleet/server/services/agent_policy.ts | 12 ++-- .../security/message_signing_service.ts | 16 +++-- .../uninstall_token_service/index.test.ts | 72 ++++++++++++++----- .../security/uninstall_token_service/index.ts | 49 ++++++++++--- x-pack/plugins/fleet/server/services/setup.ts | 34 ++++++--- 6 files changed, 139 insertions(+), 48 deletions(-) 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 b6950ba672817..931168f545b55 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.test.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.test.ts @@ -671,7 +671,9 @@ describe('agent policy', () => { jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(true); mockedAppContextService.getUninstallTokenService.mockReturnValueOnce({ - checkTokenValidityForPolicy: jest.fn().mockRejectedValueOnce(new Error('reason')), + checkTokenValidityForPolicy: jest + .fn() + .mockResolvedValueOnce({ error: new Error('reason') }), } as unknown as UninstallTokenServiceInterface); const soClient = getAgentPolicyCreateMock(); diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index 5e8c897d5611a..568829fda978e 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -1220,10 +1220,14 @@ class AgentPolicyService { 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}`); + const uninstallTokenError = await uninstallTokenService?.checkTokenValidityForPolicy( + policyId + ); + + if (uninstallTokenError) { + throw new Error( + `Cannot enable Agent Tamper Protection: ${uninstallTokenError.error.message}` + ); } } } diff --git a/x-pack/plugins/fleet/server/services/security/message_signing_service.ts b/x-pack/plugins/fleet/server/services/security/message_signing_service.ts index 77d2a2a272caf..b3d08e9a0b8e9 100644 --- a/x-pack/plugins/fleet/server/services/security/message_signing_service.ts +++ b/x-pack/plugins/fleet/server/services/security/message_signing_service.ts @@ -5,10 +5,10 @@ * 2.0. */ -import { backOff } from 'exponential-backoff'; - import { generateKeyPairSync, createSign, randomBytes } from 'crypto'; +import { backOff } from 'exponential-backoff'; + import type { LoggerFactory, Logger } from '@kbn/core/server'; import type { KibanaRequest } from '@kbn/core-http-server'; import type { @@ -202,10 +202,10 @@ export class MessageSigningService implements MessageSigningServiceInterface { soDoc = await this.getCurrentKeyPairObj(); }, { - maxDelay: 60 * 60 * 1000, // 1 hour in milliseconds startingDelay: 1000, // 1 second + maxDelay: 3000, // 3 seconds jitter: 'full', - numOfAttempts: Infinity, + numOfAttempts: 10, retry: (_err: Error, attempt: number) => { // not logging the error since we don't control what's in the error and it might contain sensitive data // ESO already logs specific caught errors before passing the error along @@ -250,7 +250,13 @@ export class MessageSigningService implements MessageSigningServiceInterface { } | undefined > { - const currentKeyPair = await this.getCurrentKeyPairObjWithRetry(); + let currentKeyPair; + try { + currentKeyPair = await this.getCurrentKeyPairObjWithRetry(); + } catch (e) { + throw new MessageSigningError('Cannot read existing Message Signing Key pair'); + } + if (!currentKeyPair) { return; } 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 4b3be1de81632..a782fc605ccf5 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 @@ -13,6 +13,8 @@ import type { SavedObjectsClientContract } from '@kbn/core/server'; import type { EncryptedSavedObjectsClient } from '@kbn/encrypted-saved-objects-plugin/server'; import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; +import { UninstallTokenError } from '../../../../common/errors'; + import { SO_SEARCH_LIMIT } from '../../../../common'; import type { @@ -252,7 +254,7 @@ describe('UninstallTokenService', () => { mockCreatePointInTimeFinder(canEncrypt, defaultBuckets); await expect(uninstallTokenService.getTokenMetadata()).rejects.toThrowError( - 'Uninstall Token is missing creation date.' + 'Invalid uninstall token: Saved object is missing creation date.' ); }); @@ -264,7 +266,7 @@ describe('UninstallTokenService', () => { mockCreatePointInTimeFinder(canEncrypt, defaultBuckets); await expect(uninstallTokenService.getTokenMetadata()).rejects.toThrowError( - 'Uninstall Token is missing policy ID.' + 'Invalid uninstall token: Saved object is missing the policy id attribute.' ); }); }); @@ -517,60 +519,94 @@ describe('UninstallTokenService', () => { }; describe('checkTokenValidityForAllPolicies', () => { - it('resolves if all of the tokens are available', async () => { + it('returns null if all of the tokens are available', async () => { mockCreatePointInTimeFinderAsInternalUser(); await expect( uninstallTokenService.checkTokenValidityForAllPolicies() - ).resolves.not.toThrowError(); + ).resolves.toBeNull(); }); - it('rejects if any of the tokens is missing', async () => { + it('returns error 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.' - ); + ).resolves.toStrictEqual({ + error: new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing the token attribute.' + ), + }); }); - it('rejects if token decryption gives error', async () => { + it('returns error if token decryption gives error', async () => { mockCreatePointInTimeFinderAsInternalUser([okaySO, errorWithDecryptionSO2]); await expect( uninstallTokenService.checkTokenValidityForAllPolicies() - ).rejects.toThrowError('Error when reading Uninstall Token: error reason'); + ).resolves.toStrictEqual({ + error: new UninstallTokenError( + "Error when reading Uninstall Token with id 'test-so-id-two'." + ), + }); + }); + + it('throws error in case of unknown error', async () => { + esoClientMock.createPointInTimeFinderDecryptedAsInternalUser = jest + .fn() + .mockRejectedValueOnce('some error'); + + await expect( + uninstallTokenService.checkTokenValidityForAllPolicies() + ).rejects.toThrowError('Unknown error happened while checking Uninstall Tokens validity'); }); }); describe('checkTokenValidityForPolicy', () => { - it('resolves if token is available', async () => { + it('returns empty array if token is available', async () => { mockCreatePointInTimeFinderAsInternalUser(); await expect( uninstallTokenService.checkTokenValidityForPolicy(okaySO.attributes.policy_id) - ).resolves.not.toThrowError(); + ).resolves.toBeNull(); }); - it('rejects if token is missing', async () => { + it('returns error 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.' - ); + ).resolves.toStrictEqual({ + error: new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing the token attribute.' + ), + }); }); - it('rejects if token decryption gives error', async () => { + it('returns error 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'); + ).resolves.toStrictEqual({ + error: new UninstallTokenError( + "Error when reading Uninstall Token with id 'test-so-id-two'." + ), + }); + }); + + it('throws error in case of unknown error', async () => { + esoClientMock.createPointInTimeFinderDecryptedAsInternalUser = jest + .fn() + .mockRejectedValueOnce('some error'); + + await expect( + uninstallTokenService.checkTokenValidityForPolicy( + errorWithDecryptionSO2.attributes.policy_id + ) + ).rejects.toThrowError('Unknown error happened while checking Uninstall Tokens validity'); }); }); }); 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 9e03e7869c584..7a2bdedffcdc8 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 @@ -55,6 +55,10 @@ interface UninstallTokenSOAggregation { by_policy_id: AggregationsMultiBucketAggregateBase; } +export interface UninstallTokenInvalidError { + error: UninstallTokenError; +} + export interface UninstallTokenServiceInterface { /** * Get uninstall token based on its id. @@ -140,14 +144,14 @@ export interface UninstallTokenServiceInterface { * * @param policyId policy Id to check */ - checkTokenValidityForPolicy(policyId: string): Promise; + 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; + checkTokenValidityForAllPolicies(): Promise; } export class UninstallTokenService implements UninstallTokenServiceInterface { @@ -226,7 +230,7 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { const uninstallTokens: UninstallToken[] = tokenObject.map( ({ id: _id, attributes, created_at: createdAt, error }) => { if (error) { - throw new UninstallTokenError(`Error when reading Uninstall Token: ${error.message}`); + throw new UninstallTokenError(`Error when reading Uninstall Token with id '${_id}'.`); } this.assertPolicyId(attributes); @@ -489,13 +493,34 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { return this._soClient; } - public async checkTokenValidityForPolicy(policyId: string): Promise { - await this.getDecryptedTokensForPolicyIds([policyId]); + public async checkTokenValidityForPolicy( + policyId: string + ): Promise { + return await this.checkTokenValidity([policyId]); } - public async checkTokenValidityForAllPolicies(): Promise { + public async checkTokenValidityForAllPolicies(): Promise { const policyIds = await this.getAllPolicyIds(); - await this.getDecryptedTokensForPolicyIds(policyIds); + return await this.checkTokenValidity(policyIds); + } + + private async checkTokenValidity( + policyIds: string[] + ): Promise { + try { + await this.getDecryptedTokensForPolicyIds(policyIds); + } catch (error) { + if (error instanceof UninstallTokenError) { + // known errors are considered non-fatal + return { error }; + } else { + const errorMessage = 'Unknown error happened while checking Uninstall Tokens validity'; + appContextService.getLogger().error(`${errorMessage}: '${error}'`); + throw new UninstallTokenError(errorMessage); + } + } + + return null; } private get isEncryptionAvailable(): boolean { @@ -504,21 +529,25 @@ export class UninstallTokenService implements UninstallTokenServiceInterface { private assertCreatedAt(createdAt: string | undefined): asserts createdAt is string { if (!createdAt) { - throw new UninstallTokenError('Uninstall Token is missing creation date.'); + throw new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing creation date.' + ); } } private assertToken(attributes: UninstallTokenSOAttributes | undefined) { if (!attributes?.token && !attributes?.token_plain) { throw new UninstallTokenError( - 'Invalid uninstall token: Saved object is missing the `token` attribute.' + 'Invalid uninstall token: Saved object is missing the token attribute.' ); } } private assertPolicyId(attributes: UninstallTokenSOAttributes | undefined) { if (!attributes?.policy_id) { - throw new UninstallTokenError('Uninstall Token is missing policy ID.'); + throw new UninstallTokenError( + 'Invalid uninstall token: Saved object is missing the policy id attribute.' + ); } } } diff --git a/x-pack/plugins/fleet/server/services/setup.ts b/x-pack/plugins/fleet/server/services/setup.ts index 60ce6460d0ac2..e0b3bdfea0bd3 100644 --- a/x-pack/plugins/fleet/server/services/setup.ts +++ b/x-pack/plugins/fleet/server/services/setup.ts @@ -14,7 +14,7 @@ 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 { MessageSigningError } from '../../common/errors'; import { AUTO_UPDATE_PACKAGES } from '../../common/constants'; import type { PreconfigurationError } from '../../common/constants'; @@ -53,6 +53,7 @@ import { getPreconfiguredFleetServerHostFromConfig, } from './preconfiguration/fleet_server_host'; import { cleanUpOldFileIndices } from './setup/clean_old_fleet_indices'; +import type { UninstallTokenInvalidError } from './security/uninstall_token_service'; export interface SetupStatus { isInitialized: boolean; @@ -60,7 +61,8 @@ export interface SetupStatus { | PreconfigurationError | DefaultPackagesInstallationError | UpgradeManagedPackagePoliciesResult - | { error: UninstallTokenError } + | UninstallTokenInvalidError + | { error: MessageSigningError } >; } @@ -174,8 +176,6 @@ async function createSetupSideEffects( ).filter((result) => (result.errors ?? []).length > 0); stepSpan?.end(); - const nonFatalErrors = [...preconfiguredPackagesNonFatalErrors, ...packagePolicyUpgradeErrors]; - logger.debug('Upgrade Fleet package install versions'); stepSpan = apm.startSpan('Upgrade package install format version', 'preconfiguration'); await upgradePackageInstallVersion({ soClient, esClient, logger }); @@ -188,7 +188,16 @@ async function createSetupSideEffects( 'xpack.encryptedSavedObjects.encryptionKey is not configured, private key passphrase is being stored in plain text' ); } - await appContextService.getMessageSigningService()?.generateKeyPair(); + let messageSigningServiceNonFatalError: { error: MessageSigningError } | undefined; + try { + await appContextService.getMessageSigningService()?.generateKeyPair(); + } catch (error) { + if (error instanceof MessageSigningError) { + messageSigningServiceNonFatalError = { error }; + } else { + throw error; + } + } logger.debug('Generating Agent uninstall tokens'); if (!appContextService.getEncryptedSavedObjectsSetup()?.canEncrypt) { @@ -204,11 +213,9 @@ async function createSetupSideEffects( } logger.debug('Checking validity of Uninstall Tokens'); - try { - await appContextService.getUninstallTokenService()?.checkTokenValidityForAllPolicies(); - } catch (error) { - nonFatalErrors.push({ error }); - } + const uninstallTokenError = await appContextService + .getUninstallTokenService() + ?.checkTokenValidityForAllPolicies(); stepSpan?.end(); stepSpan = apm.startSpan('Upgrade agent policy schema', 'preconfiguration'); @@ -222,6 +229,13 @@ async function createSetupSideEffects( await ensureDefaultEnrollmentAPIKeysExists(soClient, esClient); stepSpan?.end(); + const nonFatalErrors = [ + ...preconfiguredPackagesNonFatalErrors, + ...packagePolicyUpgradeErrors, + ...(messageSigningServiceNonFatalError ? [messageSigningServiceNonFatalError] : []), + ...(uninstallTokenError ? [uninstallTokenError] : []), + ]; + if (nonFatalErrors.length > 0) { logger.info('Encountered non fatal errors during Fleet setup'); formatNonFatalErrors(nonFatalErrors)