[Security Solution] Better threshold rule error checking (#131088)

* Better threshold rule error checking

* Add more type dependent checks

* create/import/update/add-prepackaged

* Fix tests

* whoops

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Madison Caldwell 2022-05-12 11:37:28 -04:00 committed by GitHub
parent 0b162b6881
commit cddd41dfae
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 469 additions and 68 deletions

View file

@ -91,4 +91,36 @@ describe('add_prepackaged_rules_type_dependents', () => {
const errors = addPrepackagedRuleValidateTypeDependents(schema);
expect(errors).toEqual(['"threshold.value" has to be bigger than 0']);
});
test('threshold.field should contain 3 items or less', () => {
const schema: AddPrepackagedRulesSchema = {
...getAddPrepackagedRulesSchemaMock(),
type: 'threshold',
threshold: {
field: ['field-1', 'field-2', 'field-3', 'field-4'],
value: 1,
},
};
const errors = addPrepackagedRuleValidateTypeDependents(schema);
expect(errors).toEqual(['Number of fields must be 3 or less']);
});
test('threshold.cardinality[0].field should not be in threshold.field', () => {
const schema: AddPrepackagedRulesSchema = {
...getAddPrepackagedRulesSchemaMock(),
type: 'threshold',
threshold: {
field: ['field-1', 'field-2', 'field-3'],
value: 1,
cardinality: [
{
field: 'field-1',
value: 2,
},
],
},
};
const errors = addPrepackagedRuleValidateTypeDependents(schema);
expect(errors).toEqual(['Cardinality of a field that is being aggregated on is always 1']);
});
});

View file

@ -96,29 +96,39 @@ export const validateTimelineTitle = (rule: AddPrepackagedRulesSchema): string[]
};
export const validateThreshold = (rule: AddPrepackagedRulesSchema): string[] => {
const errors: string[] = [];
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
return ['"threshold.value" has to be bigger than 0'];
errors.push('when "type" is "threshold", "threshold" is required');
} else {
return [];
if (rule.threshold.value <= 0) {
errors.push('"threshold.value" has to be bigger than 0');
}
if (
rule.threshold.cardinality?.length &&
rule.threshold.field.includes(rule.threshold.cardinality[0].field)
) {
errors.push('Cardinality of a field that is being aggregated on is always 1');
}
if (Array.isArray(rule.threshold.field) && rule.threshold.field.length > 3) {
errors.push('Number of fields must be 3 or less');
}
}
}
return [];
return errors;
};
export const addPrepackagedRuleValidateTypeDependents = (
schema: AddPrepackagedRulesSchema
rule: AddPrepackagedRulesSchema
): string[] => {
return [
...validateAnomalyThreshold(schema),
...validateQuery(schema),
...validateLanguage(schema),
...validateSavedId(schema),
...validateMachineLearningJobId(schema),
...validateTimelineId(schema),
...validateTimelineTitle(schema),
...validateThreshold(schema),
...validateAnomalyThreshold(rule),
...validateQuery(rule),
...validateLanguage(rule),
...validateSavedId(rule),
...validateMachineLearningJobId(rule),
...validateTimelineId(rule),
...validateTimelineTitle(rule),
...validateThreshold(rule),
];
};

View file

@ -34,22 +34,43 @@ export const validateTimelineTitle = (rule: CreateRulesSchema): string[] => {
};
export const validateThreatMapping = (rule: CreateRulesSchema): string[] => {
let errors: string[] = [];
const errors: string[] = [];
if (rule.type === 'threat_match') {
if (rule.concurrent_searches == null && rule.items_per_search != null) {
errors = ['when "items_per_search" exists, "concurrent_searches" must also exist', ...errors];
}
if (rule.concurrent_searches != null && rule.items_per_search == null) {
errors = ['when "concurrent_searches" exists, "items_per_search" must also exist', ...errors];
errors.push('when "concurrent_searches" exists, "items_per_search" must also exist');
}
if (rule.concurrent_searches == null && rule.items_per_search != null) {
errors.push('when "items_per_search" exists, "concurrent_searches" must also exist');
}
}
return errors;
};
export const createRuleValidateTypeDependents = (schema: CreateRulesSchema): string[] => {
export const validateThreshold = (rule: CreateRulesSchema): string[] => {
const errors: string[] = [];
if (rule.type === 'threshold') {
if (!rule.threshold) {
errors.push('when "type" is "threshold", "threshold" is required');
} else {
if (
rule.threshold.cardinality?.length &&
rule.threshold.field.includes(rule.threshold.cardinality[0].field)
) {
errors.push('Cardinality of a field that is being aggregated on is always 1');
}
if (Array.isArray(rule.threshold.field) && rule.threshold.field.length > 3) {
errors.push('Number of fields must be 3 or less');
}
}
}
return errors;
};
export const createRuleValidateTypeDependents = (rule: CreateRulesSchema): string[] => {
return [
...validateTimelineId(schema),
...validateTimelineTitle(schema),
...validateThreatMapping(schema),
...validateTimelineId(rule),
...validateTimelineTitle(rule),
...validateThreatMapping(rule),
...validateThreshold(rule),
];
};

View file

@ -75,17 +75,4 @@ describe('import_rules_type_dependents', () => {
const errors = importRuleValidateTypeDependents(schema);
expect(errors).toEqual(['when "type" is "threshold", "threshold" is required']);
});
test('threshold.value is required and has to be bigger than 0 when type is threshold and validates with it', () => {
const schema: ImportRulesSchema = {
...getImportRulesSchemaMock(),
type: 'threshold',
threshold: {
field: '',
value: -1,
},
};
const errors = importRuleValidateTypeDependents(schema);
expect(errors).toEqual(['"threshold.value" has to be bigger than 0']);
});
});

View file

@ -96,27 +96,34 @@ export const validateTimelineTitle = (rule: ImportRulesSchema): string[] => {
};
export const validateThreshold = (rule: ImportRulesSchema): string[] => {
const errors: string[] = [];
if (isThresholdRule(rule.type)) {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
return ['"threshold.value" has to be bigger than 0'];
errors.push('when "type" is "threshold", "threshold" is required');
} else {
return [];
if (
rule.threshold.cardinality?.length &&
rule.threshold.field.includes(rule.threshold.cardinality[0].field)
) {
errors.push('Cardinality of a field that is being aggregated on is always 1');
}
if (Array.isArray(rule.threshold.field) && rule.threshold.field.length > 3) {
errors.push('Number of fields must be 3 or less');
}
}
}
return [];
return errors;
};
export const importRuleValidateTypeDependents = (schema: ImportRulesSchema): string[] => {
export const importRuleValidateTypeDependents = (rule: ImportRulesSchema): string[] => {
return [
...validateAnomalyThreshold(schema),
...validateQuery(schema),
...validateLanguage(schema),
...validateSavedId(schema),
...validateMachineLearningJobId(schema),
...validateTimelineId(schema),
...validateTimelineTitle(schema),
...validateThreshold(schema),
...validateAnomalyThreshold(rule),
...validateQuery(rule),
...validateLanguage(rule),
...validateSavedId(rule),
...validateMachineLearningJobId(rule),
...validateTimelineId(rule),
...validateTimelineTitle(rule),
...validateThreshold(rule),
];
};

View file

@ -101,4 +101,36 @@ describe('patch_rules_type_dependents', () => {
const errors = patchRuleValidateTypeDependents(schema);
expect(errors).toEqual(['"threshold.value" has to be bigger than 0']);
});
test('threshold.field should contain 3 items or less', () => {
const schema: PatchRulesSchema = {
...getPatchRulesSchemaMock(),
type: 'threshold',
threshold: {
field: ['field-1', 'field-2', 'field-3', 'field-4'],
value: 1,
},
};
const errors = patchRuleValidateTypeDependents(schema);
expect(errors).toEqual(['Number of fields must be 3 or less']);
});
test('threshold.cardinality[0].field should not be in threshold.field', () => {
const schema: PatchRulesSchema = {
...getPatchRulesSchemaMock(),
type: 'threshold',
threshold: {
field: ['field-1', 'field-2', 'field-3'],
value: 1,
cardinality: [
{
field: 'field-1',
value: 2,
},
],
},
};
const errors = patchRuleValidateTypeDependents(schema);
expect(errors).toEqual(['Cardinality of a field that is being aggregated on is always 1']);
});
});

View file

@ -6,7 +6,6 @@
*/
import { isMlRule } from '../../../machine_learning/helpers';
import { isThresholdRule } from '../../utils';
import { PatchRulesSchema } from './patch_rules_schema';
export const validateQuery = (rule: PatchRulesSchema): string[] => {
@ -70,25 +69,35 @@ export const validateId = (rule: PatchRulesSchema): string[] => {
};
export const validateThreshold = (rule: PatchRulesSchema): string[] => {
if (isThresholdRule(rule.type)) {
const errors: string[] = [];
if (rule.type === 'threshold') {
if (!rule.threshold) {
return ['when "type" is "threshold", "threshold" is required'];
} else if (rule.threshold.value <= 0) {
return ['"threshold.value" has to be bigger than 0'];
errors.push('when "type" is "threshold", "threshold" is required');
} else {
return [];
if (
rule.threshold.cardinality?.length &&
rule.threshold.field.includes(rule.threshold.cardinality[0].field)
) {
errors.push('Cardinality of a field that is being aggregated on is always 1');
}
if (rule.threshold.value <= 0) {
errors.push('"threshold.value" has to be bigger than 0');
}
if (Array.isArray(rule.threshold.field) && rule.threshold.field.length > 3) {
errors.push('Number of fields must be 3 or less');
}
}
}
return [];
return errors;
};
export const patchRuleValidateTypeDependents = (schema: PatchRulesSchema): string[] => {
export const patchRuleValidateTypeDependents = (rule: PatchRulesSchema): string[] => {
return [
...validateId(schema),
...validateQuery(schema),
...validateLanguage(schema),
...validateTimelineId(schema),
...validateTimelineTitle(schema),
...validateThreshold(schema),
...validateId(rule),
...validateQuery(rule),
...validateLanguage(rule),
...validateTimelineId(rule),
...validateTimelineTitle(rule),
...validateThreshold(rule),
];
};

View file

@ -43,6 +43,31 @@ export const validateId = (rule: UpdateRulesSchema): string[] => {
}
};
export const updateRuleValidateTypeDependents = (schema: UpdateRulesSchema): string[] => {
return [...validateId(schema), ...validateTimelineId(schema), ...validateTimelineTitle(schema)];
export const validateThreshold = (rule: UpdateRulesSchema): string[] => {
const errors: string[] = [];
if (rule.type === 'threshold') {
if (!rule.threshold) {
errors.push('when "type" is "threshold", "threshold" is required');
} else {
if (
rule.threshold.cardinality?.length &&
rule.threshold.field.includes(rule.threshold.cardinality[0].field)
) {
errors.push('Cardinality of a field that is being aggregated on is always 1');
}
if (Array.isArray(rule.threshold.field) && rule.threshold.field.length > 3) {
errors.push('Number of fields must be 3 or less');
}
}
}
return errors;
};
export const updateRuleValidateTypeDependents = (rule: UpdateRulesSchema): string[] => {
return [
...validateId(rule),
...validateTimelineId(rule),
...validateTimelineTitle(rule),
...validateThreshold(rule),
];
};

View file

@ -22,7 +22,6 @@ const buildRuleMessage = buildRuleMessageFactory({
const queryFilter = getQueryFilter('', 'kuery', [], ['*'], []);
const mockSingleSearchAfter = jest.fn();
// Failing with rule registry enabled
describe('findThresholdSignals', () => {
let mockService: RuleExecutorServicesMock;

View file

@ -30,6 +30,7 @@ import {
getRuleForSignalTestingWithTimestampOverride,
waitForAlertToComplete,
waitForSignalsToBePresent,
getThresholdRuleForSignalTesting,
} from '../../utils';
import { createUserAndRole, deleteUserAndRole } from '../../../common/services/security_solution';
@ -269,6 +270,89 @@ export default ({ getService }: FtrProviderContext) => {
.expect(403);
});
});
describe('threshold validation', () => {
it('should result in 400 error if no threshold-specific fields are provided', async () => {
const { threshold, ...rule } = getThresholdRuleForSignalTesting(['*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(rule)
.expect(400);
expect(body).to.eql({
error: 'Bad Request',
message: '[request body]: Invalid value "undefined" supplied to "threshold"',
statusCode: 400,
});
});
it('should result in 400 error if more than 3 threshold fields', async () => {
const rule = getThresholdRuleForSignalTesting(['*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send({
...rule,
threshold: {
...rule.threshold,
field: ['field-1', 'field-2', 'field-3', 'field-4'],
},
})
.expect(400);
expect(body).to.eql({
message: ['Number of fields must be 3 or less'],
status_code: 400,
});
});
it('should result in 400 error if threshold value is less than 1', async () => {
const rule = getThresholdRuleForSignalTesting(['*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send({
...rule,
threshold: {
...rule.threshold,
value: 0,
},
})
.expect(400);
expect(body).to.eql({
error: 'Bad Request',
message: '[request body]: Invalid value "0" supplied to "threshold,value"',
statusCode: 400,
});
});
it('should result in 400 error if cardinality is also an agg field', async () => {
const rule = getThresholdRuleForSignalTesting(['*']);
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send({
...rule,
threshold: {
...rule.threshold,
cardinality: [
{
field: 'process.name',
value: 5,
},
],
},
})
.expect(400);
expect(body).to.eql({
message: ['Cardinality of a field that is being aggregated on is always 1'],
status_code: 400,
});
});
});
});
describe('missing timestamps', () => {

View file

@ -7,6 +7,7 @@
import expect from '@kbn/expect';
import { CreateRulesSchema } from '@kbn/security-solution-plugin/common/detection_engine/schemas/request';
import { EXCEPTION_LIST_ITEM_URL, EXCEPTION_LIST_URL } from '@kbn/securitysolution-list-constants';
import { getCreateExceptionListMinimalSchemaMock } from '@kbn/lists-plugin/common/schemas/request/create_exception_list_schema.mock';
import { DETECTION_ENGINE_RULES_URL } from '@kbn/security-solution-plugin/common/constants';
@ -24,6 +25,7 @@ import {
getSimpleRule,
getSimpleRuleAsNdjson,
getSimpleRuleOutput,
getThresholdRuleForSignalTesting,
getWebHookAction,
removeServerGeneratedProperties,
ruleToNdjson,
@ -218,6 +220,103 @@ export default ({ getService }: FtrProviderContext): void => {
});
});
});
describe('threshold validation', () => {
it('should result in partial success if no threshold-specific fields are provided', async () => {
const { threshold, ...rule } = getThresholdRuleForSignalTesting(['*']);
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', ruleToNdjson(rule as CreateRulesSchema), 'rules.ndjson')
.expect(200);
expect(body.errors[0]).to.eql({
rule_id: '(unknown id)',
error: {
message: 'when "type" is "threshold", "threshold" is required',
status_code: 400,
},
});
});
it('should result in partial success if more than 3 threshold fields', async () => {
const baseRule = getThresholdRuleForSignalTesting(['*']);
const rule = {
...baseRule,
threshold: {
...baseRule.threshold,
field: ['field-1', 'field-2', 'field-3', 'field-4'],
},
};
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', ruleToNdjson(rule), 'rules.ndjson')
.expect(200);
expect(body.errors[0]).to.eql({
rule_id: '(unknown id)',
error: {
message: 'Number of fields must be 3 or less',
status_code: 400,
},
});
});
it('should result in partial success if threshold value is less than 1', async () => {
const baseRule = getThresholdRuleForSignalTesting(['*']);
const rule = {
...baseRule,
threshold: {
...baseRule.threshold,
value: 0,
},
};
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', ruleToNdjson(rule), 'rules.ndjson')
.expect(200);
expect(body.errors[0]).to.eql({
rule_id: '(unknown id)',
error: {
message: 'Invalid value "0" supplied to "threshold,value"',
status_code: 400,
},
});
});
it('should result in 400 error if cardinality is also an agg field', async () => {
const baseRule = getThresholdRuleForSignalTesting(['*']);
const rule = {
...baseRule,
threshold: {
...baseRule.threshold,
cardinality: [
{
field: 'process.name',
value: 5,
},
],
},
};
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', ruleToNdjson(rule), 'rules.ndjson')
.expect(200);
expect(body.errors[0]).to.eql({
rule_id: '(unknown id)',
error: {
message: 'Cardinality of a field that is being aggregated on is always 1',
status_code: 400,
},
});
});
});
describe('importing rules with an index', () => {
beforeEach(async () => {
await createSignalsIndex(supertest, log);

View file

@ -24,6 +24,7 @@ import {
createRule,
getSimpleRule,
createLegacyRuleAction,
getThresholdRuleForSignalTesting,
} from '../../utils';
// eslint-disable-next-line import/no-default-export
@ -354,6 +355,101 @@ export default ({ getService }: FtrProviderContext) => {
message: 'rule_id: "fake_id" not found',
});
});
describe('threshold validation', () => {
it('should result in 400 error if no threshold-specific fields are provided', async () => {
const existingRule = getThresholdRuleForSignalTesting(['*']);
await createRule(supertest, log, existingRule);
const { threshold, ...rule } = existingRule;
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(rule)
.expect(400);
expect(body).to.eql({
error: 'Bad Request',
message: '[request body]: Invalid value "undefined" supplied to "threshold"',
statusCode: 400,
});
});
it('should result in 400 error if more than 3 threshold fields', async () => {
const existingRule = getThresholdRuleForSignalTesting(['*']);
await createRule(supertest, log, existingRule);
const rule = {
...existingRule,
threshold: {
...existingRule.threshold,
field: ['field-1', 'field-2', 'field-3', 'field-4'],
},
};
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(rule)
.expect(400);
expect(body).to.eql({
message: ['Number of fields must be 3 or less'],
status_code: 400,
});
});
it('should result in 400 error if threshold value is less than 1', async () => {
const existingRule = getThresholdRuleForSignalTesting(['*']);
await createRule(supertest, log, existingRule);
const rule = {
...existingRule,
threshold: {
...existingRule.threshold,
value: 0,
},
};
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(rule)
.expect(400);
expect(body).to.eql({
error: 'Bad Request',
message: '[request body]: Invalid value "0" supplied to "threshold,value"',
statusCode: 400,
});
});
it('should result in 400 error if cardinality is also an agg field', async () => {
const existingRule = getThresholdRuleForSignalTesting(['*']);
await createRule(supertest, log, existingRule);
const rule = {
...existingRule,
threshold: {
...existingRule.threshold,
cardinality: [
{
field: 'process.name',
value: 5,
},
],
},
};
const { body } = await supertest
.post(DETECTION_ENGINE_RULES_URL)
.set('kbn-xsrf', 'true')
.send(rule)
.expect(400);
expect(body).to.eql({
message: ['Cardinality of a field that is being aggregated on is always 1'],
status_code: 400,
});
});
});
});
});
};

View file

@ -10,7 +10,7 @@ import type { UpdateRulesSchema } from '@kbn/security-solution-plugin/common/det
/**
* This is a typical simple rule for testing that is easy for most basic testing
* @param ruleId The rule id
* @param enabled Set to tru to enable it, by default it is off
* @param enabled Set to true to enable it, by default it is off
*/
export const getSimpleRuleUpdate = (ruleId = 'rule-1', enabled = false): UpdateRulesSchema => ({
name: 'Simple Rule Query',