Skip to content

Commit

Permalink
[Entity Analytics] Asset Criticality soft delete (elastic#193591)
Browse files Browse the repository at this point in the history
## Summary

This PR introduces a "soft delete" for Asset Criticality records.
Instead of deleting the document, we now simply change the criticality
field to the value `"deleted"`.
This value is fully managed on Kibana and should not be exposed to the
client side.


### How to test

1. Add some host/user data
2. Make sure to enable the `entityStoreEnabled` feature flag
3. Assign asset criticality to a user/host.
* You may need to enable Asset Criticality in Kibana Advanced Settings
5. Verify the criticality has been assigned by `GET .asset-citicality*`.
6. Unassign the criticality for that host/user via the UI.
7. `GET`ing the criticality index should now still show the updated
record with `"deleted"` criticality value
8. The Ui should also show criticality as `Unassigned` for the deleted
record.



Implements elastic/security-team#10531, which
is part of the overall [Entity Store for
8.16](elastic/security-team#10529) work
  • Loading branch information
tiansivive authored Sep 25, 2024
1 parent baba9fe commit a8c7e06
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('AssetCriticalityDataClient', () => {
);
});

// QUESTION: This test seems useless?
it('requires a query parameter', async () => {
await subject.search({ query: { match_all: {} } });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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': {
Expand Down Expand Up @@ -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',
};
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
});
Expand Down

0 comments on commit a8c7e06

Please sign in to comment.