[8.x] [Entity Analytics] Asset Criticality soft delete (#193591) (#19…

…4010) # Backport This will backport the following commits from `main` to `8.x`: - [[Entity Analytics] Asset Criticality soft delete (#193591)](#193591)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Tiago Vila Verde <tiago.vilaverde@elastic.co>
elastic · Sep 25, 2024 · 9307e92 · 9307e92
1 parent 21805f3
commit 9307e92
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 5 deletions.
diff --git a/...ution/server/lib/entity_analytics/asset_criticality/asset_criticality_data_client.test.ts b/...ution/server/lib/entity_analytics/asset_criticality/asset_criticality_data_client.test.ts
@@ -88,6 +88,7 @@ describe('AssetCriticalityDataClient', () => {
       );
     });
 
+    // QUESTION: This test seems useless?
     it('requires a query parameter', async () => {
       await subject.search({ query: { match_all: {} } });
 

diff --git a/...y_solution/server/lib/entity_analytics/asset_criticality/asset_criticality_data_client.ts b/...y_solution/server/lib/entity_analytics/asset_criticality/asset_criticality_data_client.ts
@@ -10,14 +10,16 @@ import type { Logger, ElasticsearchClient } from '@kbn/core/server';
 import { mappingFromFieldMap } from '@kbn/alerting-plugin/common';
 import type { AuditLogger } from '@kbn/security-plugin-types-server';
 import { fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query';
+
 import type {
   BulkUpsertAssetCriticalityRecordsResponse,
   AssetCriticalityUpsert,
 } from '../../../../common/entity_analytics/asset_criticality/types';
 import type { AssetCriticalityRecord } from '../../../../common/api/entity_analytics';
 import { createOrUpdateIndex } from '../utils/create_or_update_index';
 import { getAssetCriticalityIndex } from '../../../../common/entity_analytics/asset_criticality';
-import { assetCriticalityFieldMap } from './constants';
+import type { CriticalityValues } from './constants';
+import { CRITICALITY_VALUES, assetCriticalityFieldMap } from './constants';
 import { AssetCriticalityAuditActions } from './audit';
 import { AUDIT_CATEGORY, AUDIT_OUTCOME, AUDIT_TYPE } from '../audit';
 
@@ -34,6 +36,12 @@ type BulkUpsertFromStreamOptions = {
   recordsStream: NodeJS.ReadableStream;
 } & Pick<Parameters<ElasticsearchClient['helpers']['bulk']>[0], 'flushBytes' | 'retries'>;
 
+type StoredAssetCriticalityRecord = {
+  [K in keyof AssetCriticalityRecord]: K extends 'criticality_level'
+    ? CriticalityValues
+    : AssetCriticalityRecord[K];
+};
+
 const MAX_CRITICALITY_RESPONSE_SIZE = 100_000;
 const DEFAULT_CRITICALITY_RESPONSE_SIZE = 1_000;
 
@@ -91,6 +99,13 @@ export class AssetCriticalityDataClient {
       size: Math.min(size, MAX_CRITICALITY_RESPONSE_SIZE),
       from,
       sort,
+      post_filter: {
+        bool: {
+          must_not: {
+            term: { criticality_level: CRITICALITY_VALUES.DELETED },
+          },
+        },
+      },
     });
     return response;
   }
@@ -154,12 +169,16 @@ export class AssetCriticalityDataClient {
     const id = createId(idParts);
 
     try {
-      const body = await this.options.esClient.get<AssetCriticalityRecord>({
+      const { _source: doc } = await this.options.esClient.get<StoredAssetCriticalityRecord>({
         id,
         index: this.getIndex(),
       });
 
-      return body._source;
+      if (doc?.criticality_level === CRITICALITY_VALUES.DELETED) {
+        return undefined;
+      }
+
+      return doc as AssetCriticalityRecord;
     } catch (err) {
       if (err.statusCode === 404) {
         return undefined;
@@ -292,10 +311,13 @@ export class AssetCriticalityDataClient {
     }
 
     try {
-      await this.options.esClient.delete({
+      await this.options.esClient.update({
         id: createId(idParts),
         index: this.getIndex(),
         refresh: refresh ?? false,
+        doc: {
+          criticality_level: 'deleted',
+        },
       });
     } catch (err) {
       this.options.logger.error(`Failed to delete asset criticality record: ${err.message}`);

diff --git a/x-pack/plugins/security_solution/server/lib/entity_analytics/asset_criticality/constants.ts b/x-pack/plugins/security_solution/server/lib/entity_analytics/asset_criticality/constants.ts
@@ -6,6 +6,9 @@
  */
 
 import type { FieldMap } from '@kbn/alerts-as-data-utils';
+import type { AssetCriticalityRecord } from '../../../../common/api/entity_analytics';
+
+export type CriticalityValues = AssetCriticalityRecord['criticality_level'] | 'deleted';
 
 export const assetCriticalityFieldMap: FieldMap = {
   '@timestamp': {
@@ -34,3 +37,11 @@ export const assetCriticalityFieldMap: FieldMap = {
     required: false,
   },
 } as const;
+
+export const CRITICALITY_VALUES: { readonly [K in CriticalityValues as Uppercase<K>]: K } = {
+  LOW_IMPACT: 'low_impact',
+  MEDIUM_IMPACT: 'medium_impact',
+  HIGH_IMPACT: 'high_impact',
+  EXTREME_IMPACT: 'extreme_impact',
+  DELETED: 'deleted',
+};
diff --git a/...test_suites/entity_analytics/risk_engine/trial_license_complete_tier/asset_criticality.ts b/...test_suites/entity_analytics/risk_engine/trial_license_complete_tier/asset_criticality.ts
@@ -461,7 +461,9 @@ export default ({ getService }: FtrProviderContext) => {
           es,
         });
 
-        expect(doc).to.eql(undefined);
+        const deletedDoc = { ...assetCriticality, criticality_level: 'deleted' };
+
+        expect(_.omit(doc, '@timestamp')).to.eql(deletedDoc);
       });
 
       it('should not return 404 if the asset criticality does not exist', async () => {

diff --git a/...entity_analytics/risk_engine/trial_license_complete_tier/risk_score_entity_calculation.ts b/...entity_analytics/risk_engine/trial_license_complete_tier/risk_score_entity_calculation.ts
@@ -206,6 +206,49 @@ export default ({ getService }: FtrProviderContext): void => {
             persistedScoreByApi.calculated_score_norm! + 0.000000000000001
           );
         });
+
+        it('ignores deleted asset criticality when calculating and persisting risk scores with additional criticality metadata and modifiers', async () => {
+          const documentId = uuidv4();
+          await assetCriticalityRoutes.delete('host.name', 'host-1');
+          await indexListOfDocuments([buildDocument({ host: { name: 'host-1' } }, documentId)]);
+          await waitForAssetCriticalityToBePresent({ es, log });
+          await createRuleAndWaitExecution(documentId);
+          await riskEngineRoutes.init();
+          await waitForRiskScoresToBePresent({ es, log, scoreCount: 1 });
+
+          const results = await calculateEntityRiskScore('host-1');
+
+          const expectedScore = {
+            calculated_level: 'Unknown',
+            calculated_score: 21,
+            calculated_score_norm: 8.10060175898781,
+            category_1_score: 8.10060175898781,
+            category_1_count: 1,
+            id_field: 'host.name',
+            id_value: 'host-1',
+          };
+
+          const [score] = sanitizeScores([results.score]);
+          expect(results.success).to.be(true);
+          expect(score).to.eql(expectedScore);
+
+          await waitForRiskScoresToBePresent({ es, log, scoreCount: 2 });
+          const persistedScores = await readRiskScores(es);
+
+          expect(persistedScores.length).to.greaterThan(1); // the risk score is calculated once by the risk engine and a second time by the API
+          const [persistedScoreByApi, persistedScoreByEngine] = normalizeScores(persistedScores);
+          expect(persistedScoreByApi).to.eql(expectedScore);
+          expect(persistedScoreByApi).to.eql(persistedScoreByEngine);
+
+          const [rawScore] = persistedScores;
+
+          expect(
+            rawScore.host?.risk.category_1_score! + rawScore.host?.risk.category_2_score!
+          ).to.be.within(
+            persistedScoreByApi.calculated_score_norm! - 0.000000000000001,
+            persistedScoreByApi.calculated_score_norm! + 0.000000000000001
+          );
+        });
       });
     });
   });

diff --git a/...ity_analytics/risk_engine/trial_license_complete_tier/risk_scoring_task/task_execution.ts b/...ity_analytics/risk_engine/trial_license_complete_tier/risk_scoring_task/task_execution.ts
@@ -283,6 +283,33 @@ export default ({ getService }: FtrProviderContext): void => {
               },
             ]);
           });
+
+          it('filters out deleted asset criticality data when calculating score', async () => {
+            await assetCriticalityRoutes.upsert({
+              id_field: 'host.name',
+              id_value: 'host-2',
+              criticality_level: 'high_impact',
+            });
+            await assetCriticalityRoutes.delete('host.name', 'host-2');
+            await waitForAssetCriticalityToBePresent({ es, log });
+            await riskEngineRoutes.init();
+            await waitForRiskScoresToBePresent({ es, log, scoreCount: 20 });
+            const riskScores = await readRiskScores(es);
+
+            expect(riskScores.length).to.be.greaterThan(0);
+            const assetCriticalityLevels = riskScores.map(
+              (riskScore) => riskScore.host?.risk.criticality_level
+            );
+            const assetCriticalityModifiers = riskScores.map(
+              (riskScore) => riskScore.host?.risk.criticality_modifier
+            );
+
+            expect(assetCriticalityLevels).to.not.contain('deleted');
+            expect(assetCriticalityModifiers).to.contain(2);
+
+            const scoreWithCriticality = riskScores.find((score) => score.host?.name === 'host-2');
+            expect(normalizeScores([scoreWithCriticality!])[0].criticality_level).to.be(undefined);
+          });
         });
       });
     });