Skip to content

Commit

Permalink
[Security Solution][Detections] Indicator path followup (#91747) (#91772
Browse files Browse the repository at this point in the history 
)

* Remove eslint issue by not permuting our input

We instead return a new object from our enrich function.

* Fixes editing of non-indicator rules

If the user edits a rule without visiting the About tab, they will
receive a value of threatIndicatorPath: '' which we'll then try to send
to the backend, but it gets rejected.

By removing this defaulting logic we get the correct behavior: existing
rules default to threatIndicatorPath: undefined, which gets stripped
before being sent to the backend. If the rule is an indicator rule, the
value will be persisted as expected.

* Move default indicator path to common constant
  • Loading branch information
@rylnd
rylnd authored Feb 18, 2021
1 parent dd94a51 commit 7dddca8
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 40 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ export const DEFAULT_RULE_REFRESH_INTERVAL_VALUE = 60000; // ms
export const DEFAULT_RULE_REFRESH_IDLE_VALUE = 2700000; // ms
export const DEFAULT_RULE_NOTIFICATION_QUERY_SIZE = 100;

// Document path where threat indicator fields are expected. Used as
// both the source of enrichment fields and the destination for enrichment in
// the generated detection alert
export const DEFAULT_INDICATOR_PATH = 'threat.indicator';

export enum SecurityPageName {
detections = 'detections',
overview = 'overview',
Expand Down
3 changes: 2 additions & 1 deletion ...ck/plugins/security_solution/common/detection_engine/schemas/request/rule_schemas.mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { DEFAULT_INDICATOR_PATH } from '../../../constants';
import {
MachineLearningCreateSchema,
MachineLearningUpdateSchema,
Expand Down Expand Up @@ -56,7 +57,7 @@ export const getCreateThreatMatchRulesSchemaMock = (
rule_id: ruleId,
threat_query: '*:*',
threat_index: ['list-index'],
threat_indicator_path: 'threat.indicator',
threat_indicator_path: DEFAULT_INDICATOR_PATH,
threat_mapping: [
{
entries: [
Expand Down
3 changes: 2 additions & 1 deletion .../plugins/security_solution/common/detection_engine/schemas/response/rules_schema.mocks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { DEFAULT_INDICATOR_PATH } from '../../../constants';
import { getListArrayMock } from '../types/lists.mock';

import { RulesSchema } from './rules_schema';
Expand Down Expand Up @@ -150,7 +151,7 @@ export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<Rul
language: 'kuery',
threat_query: '*:*',
threat_index: ['list-index'],
threat_indicator_path: 'threat.indicator',
threat_indicator_path: DEFAULT_INDICATOR_PATH,
threat_mapping: [
{
entries: [
Expand Down
1 change: 0 additions & 1 deletion ...ins/security_solution/public/detections/components/rules/step_about_rule/default_value.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const stepAboutDefaultValue: AboutStepRule = {
license: '',
ruleNameOverride: '',
tags: [],
threatIndicatorPath: '',
timestampOverride: '',
threat: threatDefault,
note: '',
Expand Down
3 changes: 2 additions & 1 deletion ...ck/plugins/security_solution/public/detections/components/rules/step_about_rule/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { RiskScoreField } from '../risk_score_mapping';
import { AutocompleteField } from '../autocomplete_field';
import { useFetchIndex } from '../../../../common/containers/source';
import { isThreatMatchRule } from '../../../../../common/detection_engine/utils';
import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';

const CommonUseField = getUseField({ component: Field });

Expand Down Expand Up @@ -309,7 +310,7 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
euiFieldProps: {
fullWidth: true,
disabled: isLoading,
placeholder: 'threat.indicator',
placeholder: DEFAULT_INDICATOR_PATH,
},
}}
/>
Expand Down
1 change: 0 additions & 1 deletion ...plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ describe('rule helpers', () => {
severity: { value: 'low', mapping: fillEmptySeverityMappings([]), isMappingChecked: false },
tags: ['tag1', 'tag2'],
threat: getThreatMock(),
threatIndicatorPath: '',
timestampOverride: 'event.ingested',
};
const scheduleRuleStepData = { from: '0s', interval: '5m' };
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/helpers.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export const getAboutStepsData = (rule: Rule, detailsView: boolean): AboutStepRu
},
falsePositives,
threat: threat as Threats,
threatIndicatorPath: threatIndicatorPath ?? '',
threatIndicatorPath,
};
};

Expand Down
3 changes: 1 addition & 2 deletions ...ty_solution/server/lib/detection_engine/signals/threat_mapping/build_threat_enrichment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
* 2.0.
*/

import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';
import { SignalSearchResponse, SignalsEnrichment } from '../types';
import { enrichSignalThreatMatches } from './enrich_signal_threat_matches';
import { getThreatList } from './get_threat_list';
import { BuildThreatEnrichmentOptions, GetMatchedThreats } from './types';

const DEFAULT_INDICATOR_PATH = 'threat.indicator';

export const buildThreatEnrichment = ({
buildRuleMessage,
exceptionItems,
Expand Down
45 changes: 23 additions & 22 deletions ...n/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { get } from 'lodash';
import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';

import { getThreatListItemMock } from './build_threat_mapping_filter.mock';
import {
Expand Down Expand Up @@ -93,7 +94,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries: [],
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toEqual([]);
Expand All @@ -103,7 +104,7 @@ describe('buildMatchedIndicator', () => {
const [indicator] = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(get(indicator, 'matched.atomic')).toEqual('domain_1');
Expand All @@ -113,7 +114,7 @@ describe('buildMatchedIndicator', () => {
const [indicator] = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(get(indicator, 'matched.field')).toEqual('event.field');
Expand All @@ -123,7 +124,7 @@ describe('buildMatchedIndicator', () => {
const [indicator] = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(get(indicator, 'matched.type')).toEqual('type_1');
Expand Down Expand Up @@ -152,7 +153,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toHaveLength(queries.length);
Expand All @@ -162,7 +163,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toEqual([
Expand Down Expand Up @@ -227,7 +228,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toEqual([
Expand All @@ -252,7 +253,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toEqual([
Expand Down Expand Up @@ -284,7 +285,7 @@ describe('buildMatchedIndicator', () => {
const indicators = buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
});

expect(indicators).toEqual([
Expand Down Expand Up @@ -316,7 +317,7 @@ describe('buildMatchedIndicator', () => {
buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
})
).toThrowError('Expected indicator field to be an object, but found: not an object');
});
Expand All @@ -337,7 +338,7 @@ describe('buildMatchedIndicator', () => {
buildMatchedIndicator({
queries,
threats,
indicatorPath: 'threat.indicator',
indicatorPath: DEFAULT_INDICATOR_PATH,
})
).toThrowError('Expected indicator field to be an object, but found: not an object');
});
Expand Down Expand Up @@ -366,7 +367,7 @@ describe('enrichSignalThreatMatches', () => {
const enrichedSignals = await enrichSignalThreatMatches(
signals,
getMatchedThreats,
'threat.indicator'
DEFAULT_INDICATOR_PATH
);

expect(enrichedSignals.hits.hits).toEqual([]);
Expand All @@ -381,10 +382,10 @@ describe('enrichSignalThreatMatches', () => {
const enrichedSignals = await enrichSignalThreatMatches(
signals,
getMatchedThreats,
'threat.indicator'
DEFAULT_INDICATOR_PATH
);
const [enrichedHit] = enrichedSignals.hits.hits;
const indicators = get(enrichedHit._source, 'threat.indicator');
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);

expect(indicators).toEqual([
{ existing: 'indicator' },
Expand All @@ -406,10 +407,10 @@ describe('enrichSignalThreatMatches', () => {
const enrichedSignals = await enrichSignalThreatMatches(
signals,
getMatchedThreats,
'threat.indicator'
DEFAULT_INDICATOR_PATH
);
const [enrichedHit] = enrichedSignals.hits.hits;
const indicators = get(enrichedHit._source, 'threat.indicator');
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);

expect(indicators).toEqual([
{
Expand All @@ -427,10 +428,10 @@ describe('enrichSignalThreatMatches', () => {
const enrichedSignals = await enrichSignalThreatMatches(
signals,
getMatchedThreats,
'threat.indicator'
DEFAULT_INDICATOR_PATH
);
const [enrichedHit] = enrichedSignals.hits.hits;
const indicators = get(enrichedHit._source, 'threat.indicator');
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);

expect(indicators).toEqual([
{ existing: 'indicator' },
Expand All @@ -450,7 +451,7 @@ describe('enrichSignalThreatMatches', () => {
});
const signals = getSignalsResponseMock([signalHit]);
await expect(() =>
enrichSignalThreatMatches(signals, getMatchedThreats, 'threat.indicator')
enrichSignalThreatMatches(signals, getMatchedThreats, DEFAULT_INDICATOR_PATH)
).rejects.toThrowError('Expected threat field to be an object, but found: whoops');
});

Expand Down Expand Up @@ -486,7 +487,7 @@ describe('enrichSignalThreatMatches', () => {
'custom_threat.custom_indicator'
);
const [enrichedHit] = enrichedSignals.hits.hits;
const indicators = get(enrichedHit._source, 'threat.indicator');
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);

expect(indicators).toEqual([
{
Expand Down Expand Up @@ -529,13 +530,13 @@ describe('enrichSignalThreatMatches', () => {
const enrichedSignals = await enrichSignalThreatMatches(
signals,
getMatchedThreats,
'threat.indicator'
DEFAULT_INDICATOR_PATH
);
expect(enrichedSignals.hits.total).toEqual(expect.objectContaining({ value: 1 }));
expect(enrichedSignals.hits.hits).toHaveLength(1);

const [enrichedHit] = enrichedSignals.hits.hits;
const indicators = get(enrichedHit._source, 'threat.indicator');
const indicators = get(enrichedHit._source, DEFAULT_INDICATOR_PATH);

expect(indicators).toEqual([
{
Expand Down
22 changes: 12 additions & 10 deletions ...lution/server/lib/detection_engine/signals/threat_mapping/enrich_signal_threat_matches.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { get, isObject } from 'lodash';
import { DEFAULT_INDICATOR_PATH } from '../../../../../common/constants';

import type { SignalSearchResponse, SignalSourceHit } from '../types';
import type {
Expand Down Expand Up @@ -91,7 +92,7 @@ export const enrichSignalThreatMatches = async (
if (!isObject(threat)) {
throw new Error(`Expected threat field to be an object, but found: ${threat}`);
}
const existingIndicatorValue = get(signalHit._source, 'threat.indicator') ?? [];
const existingIndicatorValue = get(signalHit._source, DEFAULT_INDICATOR_PATH) ?? [];
const existingIndicators = [existingIndicatorValue].flat(); // ensure indicators is an array

return {
Expand All @@ -105,14 +106,15 @@ export const enrichSignalThreatMatches = async (
},
};
});
/* eslint-disable require-atomic-updates */
signals.hits.hits = enrichedSignals;
if (isObject(signals.hits.total)) {
signals.hits.total.value = enrichedSignals.length;
} else {
signals.hits.total = enrichedSignals.length;
}
/* eslint-enable require-atomic-updates */

return signals;
return {
...signals,
hits: {
...signals.hits,
hits: enrichedSignals,
total: isObject(signals.hits.total)
? { ...signals.hits.total, value: enrichedSignals.length }
: enrichedSignals.length,
},
};
};

0 comments on commit 7dddca8

Please sign in to comment.