Skip to content

Commit

Permalink
[Security Solution][Detections][7.12] Critical Threshold Rule Fixes (#…
Browse files Browse the repository at this point in the history 
…92667)

* Threshold cardinality validation

* Remove comments

* Fix legacy threshold signal dupe mitigation

* Add find_threshold_signals tests

* remove comment

* bug fixes

* Fix edit form value initialization for cardinality_value

* Fix test

* Type and test fixes

* Tests/types

* Reenable threshold cypress test

* Schema fixes

* Types and tests, normalize threshold field util

* Continue cleaning up types

* Some more pre-7.12 tests

* Limit cardinality_field to length 1 for now

* Cardinality to array

* Cardinality to array

* Tests/types

* cardinality can be null

* Handle empty threshold field in bulk_create_threshold_signals

* Remove cardinality_field, cardinality_value
  • Loading branch information
@madirey
madirey committed Mar 1, 2021
1 parent f249d81 commit 68fd37c
Show file tree
Hide file tree
Showing 28 changed files with 1,392 additions and 352 deletions.
48 changes: 39 additions & 9 deletions x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,26 +465,56 @@ export type Threats = t.TypeOf<typeof threats>;
export const threatsOrUndefined = t.union([threats, t.undefined]);
export type ThreatsOrUndefined = t.TypeOf<typeof threatsOrUndefined>;

export const thresholdField = t.exact(
t.type({
field: t.union([t.string, t.array(t.string)]), // Covers pre- and post-7.12
value: PositiveIntegerGreaterThanZero,
})
);
export type ThresholdField = t.TypeOf<typeof thresholdField>;

export const thresholdFieldNormalized = t.exact(
t.type({
field: t.array(t.string),
value: PositiveIntegerGreaterThanZero,
})
);
export type ThresholdFieldNormalized = t.TypeOf<typeof thresholdFieldNormalized>;

export const thresholdCardinalityField = t.exact(
t.type({
field: t.string,
value: PositiveInteger,
})
);
export type ThresholdCardinalityField = t.TypeOf<typeof thresholdCardinalityField>;

export const threshold = t.intersection([
t.exact(
t.type({
field: t.union([t.string, t.array(t.string)]),
value: PositiveIntegerGreaterThanZero,
})
),
thresholdField,
t.exact(
t.partial({
cardinality_field: t.union([t.string, t.array(t.string), t.undefined, t.null]),
cardinality_value: t.union([PositiveInteger, t.undefined, t.null]), // TODO: cardinality_value should be set if cardinality_field is set
cardinality: t.union([t.array(thresholdCardinalityField), t.null]),
})
),
]);
// TODO: codec to transform threshold field string to string[] ?
export type Threshold = t.TypeOf<typeof threshold>;

export const thresholdOrUndefined = t.union([threshold, t.undefined]);
export type ThresholdOrUndefined = t.TypeOf<typeof thresholdOrUndefined>;

export const thresholdNormalized = t.intersection([
thresholdFieldNormalized,
t.exact(
t.partial({
cardinality: t.union([t.array(thresholdCardinalityField), t.null]),
})
),
]);
export type ThresholdNormalized = t.TypeOf<typeof thresholdNormalized>;

export const thresholdNormalizedOrUndefined = t.union([thresholdNormalized, t.undefined]);
export type ThresholdNormalizedOrUndefined = t.TypeOf<typeof thresholdNormalizedOrUndefined>;

export const created_at = IsoDateString;
export const updated_at = IsoDateString;
export const updated_by = t.string;
Expand Down
26 changes: 25 additions & 1 deletion x-pack/plugins/security_solution/common/detection_engine/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
* 2.0.
*/

import { hasEqlSequenceQuery, hasLargeValueList, hasNestedEntry, isThreatMatchRule } from './utils';
import {
hasEqlSequenceQuery,
hasLargeValueList,
hasNestedEntry,
isThreatMatchRule,
normalizeThresholdField,
} from './utils';
import { EntriesArray } from '../shared_imports';

describe('#hasLargeValueList', () => {
Expand Down Expand Up @@ -151,3 +157,21 @@ describe('#hasEqlSequenceQuery', () => {
});
});
});

describe('normalizeThresholdField', () => {
it('converts a string to a string array', () => {
expect(normalizeThresholdField('host.name')).toEqual(['host.name']);
});
it('returns a string array when a string array is passed in', () => {
expect(normalizeThresholdField(['host.name'])).toEqual(['host.name']);
});
it('converts undefined to an empty array', () => {
expect(normalizeThresholdField(undefined)).toEqual([]);
});
it('converts null to an empty array', () => {
expect(normalizeThresholdField(null)).toEqual([]);
});
it('converts an empty string to an empty array', () => {
expect(normalizeThresholdField('')).toEqual([]);
});
});
12 changes: 12 additions & 0 deletions x-pack/plugins/security_solution/common/detection_engine/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { isEmpty } from 'lodash';

import {
CreateExceptionListItemSchema,
EntriesArray,
Expand Down Expand Up @@ -42,3 +44,13 @@ export const isQueryRule = (ruleType: Type | undefined): boolean =>
ruleType === 'query' || ruleType === 'saved_query';
export const isThreatMatchRule = (ruleType: Type | undefined): boolean =>
ruleType === 'threat_match';

export const normalizeThresholdField = (
thresholdField: string | string[] | null | undefined
): string[] => {
return Array.isArray(thresholdField)
? thresholdField
: isEmpty(thresholdField)
? []
: [thresholdField!];
};
6 changes: 4 additions & 2 deletions ...gins/security_solution/common/search_strategy/security_solution/matrix_histogram/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@ export interface MatrixHistogramRequestOptions extends RequestBasicOptions {
| {
field: string | string[] | undefined;
value: number;
cardinality_field?: string | undefined;
cardinality_value?: number | undefined;
cardinality?: {
field: string[];
value: number;
};
}
| undefined;
inspect?: Maybe<Inspect>;
Expand Down
177 changes: 87 additions & 90 deletions x-pack/plugins/security_solution/cypress/integration/detection_rules/threshold_rule.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,103 +79,100 @@ import { loginAndWaitForPageWithoutDateRange } from '../../tasks/login';

import { DETECTIONS_URL } from '../../urls/navigation';

// Skipped until post-FF for 7.12
describe.skip('Threshold Rules', () => {
describe('Detection rules, threshold', () => {
const expectedUrls = newThresholdRule.referenceUrls.join('');
const expectedFalsePositives = newThresholdRule.falsePositivesExamples.join('');
const expectedTags = newThresholdRule.tags.join('');
const expectedMitre = formatMitreAttackDescription(newThresholdRule.mitre);

const rule = { ...newThresholdRule };

beforeEach(() => {
cleanKibana();
createTimeline(newThresholdRule.timeline).then((response) => {
rule.timeline.id = response.body.data.persistTimeline.timeline.savedObjectId;
});
describe('Detection rules, threshold', () => {
const expectedUrls = newThresholdRule.referenceUrls.join('');
const expectedFalsePositives = newThresholdRule.falsePositivesExamples.join('');
const expectedTags = newThresholdRule.tags.join('');
const expectedMitre = formatMitreAttackDescription(newThresholdRule.mitre);

const rule = { ...newThresholdRule };

beforeEach(() => {
cleanKibana();
createTimeline(newThresholdRule.timeline).then((response) => {
rule.timeline.id = response.body.data.persistTimeline.timeline.savedObjectId;
});
});

it('Creates and activates a new threshold rule', () => {
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
waitForAlertsPanelToBeLoaded();
waitForAlertsIndexToBeCreated();
goToManageAlertsDetectionRules();
waitForRulesTableToBeLoaded();
goToCreateNewRule();
selectThresholdRuleType();
fillDefineThresholdRuleAndContinue(rule);
fillAboutRuleAndContinue(rule);
fillScheduleRuleAndContinue(rule);
createAndActivateRule();

cy.get(CUSTOM_RULES_BTN).should('have.text', 'Custom rules (1)');

changeRowsPerPageTo300();

const expectedNumberOfRules = 1;
cy.get(RULES_TABLE).then(($table) => {
cy.wrap($table.find(RULES_ROW).length).should('eql', expectedNumberOfRules);
});
it('Creates and activates a new threshold rule', () => {
loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
waitForAlertsPanelToBeLoaded();
waitForAlertsIndexToBeCreated();
goToManageAlertsDetectionRules();
waitForRulesTableToBeLoaded();
goToCreateNewRule();
selectThresholdRuleType();
fillDefineThresholdRuleAndContinue(rule);
fillAboutRuleAndContinue(rule);
fillScheduleRuleAndContinue(rule);
createAndActivateRule();

cy.get(CUSTOM_RULES_BTN).should('have.text', 'Custom rules (1)');

changeRowsPerPageTo300();

const expectedNumberOfRules = 1;
cy.get(RULES_TABLE).then(($table) => {
cy.wrap($table.find(RULES_ROW).length).should('eql', expectedNumberOfRules);
});

filterByCustomRules();
filterByCustomRules();

cy.get(RULES_TABLE).then(($table) => {
cy.wrap($table.find(RULES_ROW).length).should('eql', 1);
});
cy.get(RULE_NAME).should('have.text', rule.name);
cy.get(RISK_SCORE).should('have.text', rule.riskScore);
cy.get(SEVERITY).should('have.text', rule.severity);
cy.get(RULE_SWITCH).should('have.attr', 'aria-checked', 'true');

goToRuleDetails();

cy.get(RULE_NAME_HEADER).should('have.text', `${rule.name}`);
cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', rule.description);
cy.get(ABOUT_DETAILS).within(() => {
getDetails(SEVERITY_DETAILS).should('have.text', rule.severity);
getDetails(RISK_SCORE_DETAILS).should('have.text', rule.riskScore);
getDetails(REFERENCE_URLS_DETAILS).should((details) => {
expect(removeExternalLinkText(details.text())).equal(expectedUrls);
});
getDetails(FALSE_POSITIVES_DETAILS).should('have.text', expectedFalsePositives);
getDetails(MITRE_ATTACK_DETAILS).should((mitre) => {
expect(removeExternalLinkText(mitre.text())).equal(expectedMitre);
});
getDetails(TAGS_DETAILS).should('have.text', expectedTags);
});
cy.get(INVESTIGATION_NOTES_TOGGLE).click({ force: true });
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', rule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Threshold');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
getDetails(THRESHOLD_DETAILS).should(
'have.text',
`Results aggregated by ${rule.thresholdField} >= ${rule.threshold}`
);
cy.get(RULES_TABLE).then(($table) => {
cy.wrap($table.find(RULES_ROW).length).should('eql', 1);
});
cy.get(RULE_NAME).should('have.text', rule.name);
cy.get(RISK_SCORE).should('have.text', rule.riskScore);
cy.get(SEVERITY).should('have.text', rule.severity);
cy.get(RULE_SWITCH).should('have.attr', 'aria-checked', 'true');

goToRuleDetails();

cy.get(RULE_NAME_HEADER).should('have.text', `${rule.name}`);
cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', rule.description);
cy.get(ABOUT_DETAILS).within(() => {
getDetails(SEVERITY_DETAILS).should('have.text', rule.severity);
getDetails(RISK_SCORE_DETAILS).should('have.text', rule.riskScore);
getDetails(REFERENCE_URLS_DETAILS).should((details) => {
expect(removeExternalLinkText(details.text())).equal(expectedUrls);
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should(
'have.text',
`${rule.runsEvery.interval}${rule.runsEvery.type}`
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should(
'have.text',
`${rule.lookBack.interval}${rule.lookBack.type}`
);
getDetails(FALSE_POSITIVES_DETAILS).should('have.text', expectedFalsePositives);
getDetails(MITRE_ATTACK_DETAILS).should((mitre) => {
expect(removeExternalLinkText(mitre.text())).equal(expectedMitre);
});
getDetails(TAGS_DETAILS).should('have.text', expectedTags);
});
cy.get(INVESTIGATION_NOTES_TOGGLE).click({ force: true });
cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
cy.get(DEFINITION_DETAILS).within(() => {
getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
getDetails(CUSTOM_QUERY_DETAILS).should('have.text', rule.customQuery);
getDetails(RULE_TYPE_DETAILS).should('have.text', 'Threshold');
getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
getDetails(THRESHOLD_DETAILS).should(
'have.text',
`Results aggregated by ${rule.thresholdField} >= ${rule.threshold}`
);
});
cy.get(SCHEDULE_DETAILS).within(() => {
getDetails(RUNS_EVERY_DETAILS).should(
'have.text',
`${rule.runsEvery.interval}${rule.runsEvery.type}`
);
getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should(
'have.text',
`${rule.lookBack.interval}${rule.lookBack.type}`
);
});

waitForTheRuleToBeExecuted();
waitForAlertsToPopulate();
waitForTheRuleToBeExecuted();
waitForAlertsToPopulate();

cy.get(NUMBER_OF_ALERTS).should(($count) => expect(+$count.text()).to.be.lt(100));
cy.get(ALERT_RULE_NAME).first().should('have.text', rule.name);
cy.get(ALERT_RULE_VERSION).first().should('have.text', '1');
cy.get(ALERT_RULE_METHOD).first().should('have.text', 'threshold');
cy.get(ALERT_RULE_SEVERITY).first().should('have.text', rule.severity.toLowerCase());
cy.get(ALERT_RULE_RISK_SCORE).first().should('have.text', rule.riskScore);
});
cy.get(NUMBER_OF_ALERTS).should(($count) => expect(+$count.text()).to.be.lt(100));
cy.get(ALERT_RULE_NAME).first().should('have.text', rule.name);
cy.get(ALERT_RULE_VERSION).first().should('have.text', '1');
cy.get(ALERT_RULE_METHOD).first().should('have.text', 'threshold');
cy.get(ALERT_RULE_SEVERITY).first().should('have.text', rule.severity.toLowerCase());
cy.get(ALERT_RULE_RISK_SCORE).first().should('have.text', rule.riskScore);
});
});
6 changes: 4 additions & 2 deletions x-pack/plugins/security_solution/public/common/components/matrix_histogram/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ export interface MatrixHistogramQueryProps {
| {
field: string | string[] | undefined;
value: number;
cardinality_field?: string | undefined;
cardinality_value?: number | undefined;
cardinality?: {
field: string[];
value: number;
};
}
| undefined;
skip?: boolean;
Expand Down
24 changes: 16 additions & 8 deletions ...plugins/security_solution/public/detections/components/rules/query_preview/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,10 @@ describe('PreviewQuery', () => {
threshold={{
field: 'agent.hostname',
value: 200,
cardinality_field: 'user.name',
cardinality_value: 2,
cardinality: {
field: ['user.name'],
value: 2,
},
}}
isDisabled={false}
/>
Expand Down Expand Up @@ -338,8 +340,10 @@ describe('PreviewQuery', () => {
threshold={{
field: 'agent.hostname',
value: 200,
cardinality_field: 'user.name',
cardinality_value: 2,
cardinality: {
field: ['user.name'],
value: 2,
},
}}
isDisabled={false}
/>
Expand Down Expand Up @@ -382,8 +386,10 @@ describe('PreviewQuery', () => {
threshold={{
field: undefined,
value: 200,
cardinality_field: 'user.name',
cardinality_value: 2,
cardinality: {
field: ['user.name'],
value: 2,
},
}}
isDisabled={false}
/>
Expand Down Expand Up @@ -414,8 +420,10 @@ describe('PreviewQuery', () => {
threshold={{
field: ' ',
value: 200,
cardinality_field: 'user.name',
cardinality_value: 2,
cardinality: {
field: ['user.name'],
value: 2,
},
}}
isDisabled={false}
/>
Expand Down
Loading

0 comments on commit 68fd37c

Please sign in to comment.