Skip to content

Commit

Permalink
[Defend Workflows] Handling message signing decryption fail during Fl…
Browse files Browse the repository at this point in the history
…eet setup (elastic#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:
- elastic#171998
- elastic#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
  • Loading branch information
gergoabraham authored Dec 1, 2023
1 parent 991b5f6 commit abac134
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 48 deletions.
4 changes: 3 additions & 1 deletion x-pack/plugins/fleet/server/services/agent_policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
12 changes: 8 additions & 4 deletions x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.'
);
});

Expand All @@ -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.'
);
});
});
Expand Down Expand Up @@ -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');
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ interface UninstallTokenSOAggregation {
by_policy_id: AggregationsMultiBucketAggregateBase<UninstallTokenSOAggregationBucket>;
}

export interface UninstallTokenInvalidError {
error: UninstallTokenError;
}

export interface UninstallTokenServiceInterface {
/**
* Get uninstall token based on its id.
Expand Down Expand Up @@ -140,14 +144,14 @@ export interface UninstallTokenServiceInterface {
*
* @param policyId policy Id to check
*/
checkTokenValidityForPolicy(policyId: string): Promise<void>;
checkTokenValidityForPolicy(policyId: string): Promise<UninstallTokenInvalidError | null>;

/**
* Check whether all policies have a valid uninstall token. Rejects returning promise if not.
*
* @param policyId policy Id to check
*/
checkTokenValidityForAllPolicies(): Promise<void>;
checkTokenValidityForAllPolicies(): Promise<UninstallTokenInvalidError | null>;
}

export class UninstallTokenService implements UninstallTokenServiceInterface {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -489,13 +493,34 @@ export class UninstallTokenService implements UninstallTokenServiceInterface {
return this._soClient;
}

public async checkTokenValidityForPolicy(policyId: string): Promise<void> {
await this.getDecryptedTokensForPolicyIds([policyId]);
public async checkTokenValidityForPolicy(
policyId: string
): Promise<UninstallTokenInvalidError | null> {
return await this.checkTokenValidity([policyId]);
}

public async checkTokenValidityForAllPolicies(): Promise<void> {
public async checkTokenValidityForAllPolicies(): Promise<UninstallTokenInvalidError | null> {
const policyIds = await this.getAllPolicyIds();
await this.getDecryptedTokensForPolicyIds(policyIds);
return await this.checkTokenValidity(policyIds);
}

private async checkTokenValidity(
policyIds: string[]
): Promise<UninstallTokenInvalidError | null> {
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 {
Expand All @@ -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.'
);
}
}
}
34 changes: 24 additions & 10 deletions x-pack/plugins/fleet/server/services/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -53,14 +53,16 @@ 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;
nonFatalErrors: Array<
| PreconfigurationError
| DefaultPackagesInstallationError
| UpgradeManagedPackagePoliciesResult
| { error: UninstallTokenError }
| UninstallTokenInvalidError
| { error: MessageSigningError }
>;
}

Expand Down Expand Up @@ -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 });
Expand All @@ -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) {
Expand All @@ -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');
Expand All @@ -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)
Expand Down

0 comments on commit abac134

Please sign in to comment.