[Defend Workflows][8.12 port] Unblock fleet setup when cannot decrypt…

… uninstall tokens (elastic#172058) ## Summary This PR is the `8.12` port of: - elastic#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.
jpdjere · Nov 28, 2023 · a658233 · a658233
1 parent 177dbd1
commit a658233
Show file tree

Hide file tree

Showing 6 changed files with 189 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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<void> {
+    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
@@ -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
@@ -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<string>;
+  generateTokenForPolicyId(policyId: string, force?: boolean): Promise<void>;
 
   /**
    * 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<policyId, hashedToken>
    */
-  generateTokensForPolicyIds(policyIds: string[], force?: boolean): Promise<Record<string, string>>;
+  generateTokensForPolicyIds(policyIds: string[], force?: boolean): Promise<void>;
 
   /**
    * 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<policyId, hashedToken>
    */
-  generateTokensForAllPolicies(force?: boolean): Promise<Record<string, string>>;
+  generateTokensForAllPolicies(force?: boolean): Promise<void>;
 
   /**
    * If encryption is available, checks for any plain text uninstall tokens and encrypts them
    */
   encryptTokens(): Promise<void>;
+
+  /**
+   * 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<void>;
+
+  /**
+   * Check whether all policies have a valid uninstall token. Rejects returning promise if not.
+   *
+   * @param policyId policy Id to check
+   */
+  checkTokenValidityForAllPolicies(): Promise<void>;
 }
 
 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<string> {
-    return (await this.generateTokensForPolicyIds([policyId], force))[policyId];
+  public generateTokenForPolicyId(policyId: string, force: boolean = false): Promise<void> {
+    return this.generateTokensForPolicyIds([policyId], force);
   }
 
   public async generateTokensForPolicyIds(
     policyIds: string[],
     force: boolean = false
-  ): Promise<Record<string, string>> {
+  ): Promise<void> {
     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<string, string>
-        );
+    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<string, string>);
-
     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<string, string>);
   }
 
-  public async generateTokensForAllPolicies(
-    force: boolean = false
-  ): Promise<Record<string, string>> {
+  public async generateTokensForAllPolicies(force: boolean = false): Promise<void> {
     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<void> {
+    await this.getDecryptedTokensForPolicyIds([policyId]);
+  }
+
+  public async checkTokenValidityForAllPolicies(): Promise<void> {
+    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.'
+      );
     }
   }