Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Security Solution] [Detections] Remove file validation on import route (#77770) #77821

Merged
merged 1 commit into from
Sep 17, 2020
Merged

[7.x] [Security Solution] [Detections] Remove file validation on import route (#77770) #77821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions ...ins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ describe('import_rules_route', () => {
expect(response.status).toEqual(200);
});

test('returns 500 if more than 10,000 rules are imported', async () => {
const ruleIds = new Array(10001).fill(undefined).map((_, index) => `rule-${index}`);
const multiRequest = getImportRulesRequest(buildHapiStream(ruleIdsToNdJsonString(ruleIds)));
const response = await server.inject(multiRequest, context);

expect(response.status).toEqual(500);
expect(response.body).toEqual({
message: "Can't import more than 10000 rules",
status_code: 500,
});
});

test('returns 404 if alertClient is not available on the route', async () => {
context.alerting!.getAlertsClient = jest.fn();
const response = await server.inject(request, context);
Expand Down Expand Up @@ -229,6 +241,19 @@ describe('import_rules_route', () => {
});
});

test('returns 200 if many rules are imported successfully', async () => {
const ruleIds = new Array(9999).fill(undefined).map((_, index) => `rule-${index}`);
const multiRequest = getImportRulesRequest(buildHapiStream(ruleIdsToNdJsonString(ruleIds)));
const response = await server.inject(multiRequest, context);

expect(response.status).toEqual(200);
expect(response.body).toEqual({
errors: [],
success: true,
success_count: 9999,
});
});

test('returns 200 with errors if all rules are missing rule_ids and import fails on validation', async () => {
const rulesWithoutRuleIds = ['rule-1', 'rule-2'].map((ruleId) =>
getImportRulesWithIdSchemaMock(ruleId)
Expand Down
10 changes: 3 additions & 7 deletions .../plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@

import { chunk } from 'lodash/fp';
import { extname } from 'path';
import { schema } from '@kbn/config-schema';

import { validate } from '../../../../../common/validate';
import {
importRulesQuerySchema,
ImportRulesQuerySchemaDecoded,
importRulesPayloadSchema,
ImportRulesPayloadSchemaDecoded,
ImportRulesSchemaDecoded,
} from '../../../../../common/detection_engine/schemas/request/import_rules_schema';
import {
Expand Down Expand Up @@ -48,7 +47,7 @@ import { PartialFilter } from '../../types';

type PromiseFromStreams = ImportRulesSchemaDecoded | Error;

const CHUNK_PARSED_OBJECT_SIZE = 10;
const CHUNK_PARSED_OBJECT_SIZE = 50;

export const importRulesRoute = (router: IRouter, config: ConfigType, ml: SetupPlugins['ml']) => {
router.post(
Expand All @@ -58,10 +57,7 @@ export const importRulesRoute = (router: IRouter, config: ConfigType, ml: SetupP
query: buildRouteValidation<typeof importRulesQuerySchema, ImportRulesQuerySchemaDecoded>(
importRulesQuerySchema
),
body: buildRouteValidation<
typeof importRulesPayloadSchema,
ImportRulesPayloadSchemaDecoded
>(importRulesPayloadSchema),
body: schema.any(), // validation on file object is accomplished later in the handler.
},
options: {
tags: ['access:securitySolution'],
Expand Down
62 changes: 58 additions & 4 deletions x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1'], true), 'rules.ndjson')
.expect(200);

const { body } = await supertest
Expand Down Expand Up @@ -192,6 +192,56 @@ export default ({ getService }: FtrProviderContext): void => {
});
});

// import is very slow in 7.10+ due to the alerts client find api
// when importing 100 rules it takes about 30 seconds for this
// test to complete so at 10 rules completing in about 10 seconds
// I figured this is enough to make sure the import route is doing its job.
it('should be able to import 10 rules', async () => {
const ruleIds = new Array(10).fill(undefined).map((_, index) => `rule-${index}`);
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(ruleIds, false), 'rules.ndjson')
.expect(200);

expect(body).to.eql({
errors: [],
success: true,
success_count: 10,
});
});

// uncomment the below test once we speed up the alerts client find api
// in another PR.
// it('should be able to import 10000 rules', async () => {
// const ruleIds = new Array(10000).fill(undefined).map((_, index) => `rule-${index}`);
// const { body } = await supertest
// .post(`${DETECTION_ENGINE_RULES_URL}/_import`)
// .set('kbn-xsrf', 'true')
// .attach('file', getSimpleRuleAsNdjson(ruleIds, false), 'rules.ndjson')
// .expect(200);

// expect(body).to.eql({
// errors: [],
// success: true,
// success_count: 10000,
// });
// });

it('should NOT be able to import more than 10,000 rules', async () => {
const ruleIds = new Array(10001).fill(undefined).map((_, index) => `rule-${index}`);
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(ruleIds, false), 'rules.ndjson')
.expect(500);

expect(body).to.eql({
status_code: 500,
message: "Can't import more than 10000 rules",
});
});

it('should report a conflict if there is an attempt to import two rules with the same rule_id', async () => {
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
Expand Down Expand Up @@ -280,7 +330,7 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1'], true), 'rules.ndjson')
.expect(200);

const simpleRule = getSimpleRule('rule-1');
Expand Down Expand Up @@ -372,13 +422,17 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2'], true), 'rules.ndjson')
.expect(200);

await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2', 'rule-3']), 'rules.ndjson')
.attach(
'file',
getSimpleRuleAsNdjson(['rule-1', 'rule-2', 'rule-3'], true),
'rules.ndjson'
)
.expect(200);

const { body: bodyOfRule1 } = await supertest
Expand Down
12 changes: 8 additions & 4 deletions x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1'], true), 'rules.ndjson')
.expect(200);

const { body } = await supertest
Expand Down Expand Up @@ -243,7 +243,7 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1'], true), 'rules.ndjson')
.expect(200);

const simpleRule = getSimpleRule('rule-1');
Expand Down Expand Up @@ -335,13 +335,17 @@ export default ({ getService }: FtrProviderContext): void => {
await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2']), 'rules.ndjson')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2'], true), 'rules.ndjson')
.expect(200);

await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
.attach('file', getSimpleRuleAsNdjson(['rule-1', 'rule-2', 'rule-3']), 'rules.ndjson')
.attach(
'file',
getSimpleRuleAsNdjson(['rule-1', 'rule-2', 'rule-3'], true),
'rules.ndjson'
)
.expect(200);

const { body: bodyOfRule1 } = await supertest
Expand Down
8 changes: 5 additions & 3 deletions x-pack/test/detection_engine_api_integration/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ export const removeServerGeneratedPropertiesIncludingRuleId = (
/**
* This is a typical simple rule for testing that is easy for most basic testing
* @param ruleId
* @param enabled Enables the rule on creation or not. Defaulted to false to enable it on import
*/
export const getSimpleRule = (ruleId = 'rule-1'): CreateRulesSchema => ({
export const getSimpleRule = (ruleId = 'rule-1', enabled = true): CreateRulesSchema => ({
name: 'Simple Rule Query',
description: 'Simple Rule Query',
enabled,
risk_score: 1,
rule_id: ruleId,
severity: 'high',
Expand Down Expand Up @@ -360,9 +362,9 @@ export const deleteSignalsIndex = async (
* for testing uploads.
* @param ruleIds Array of strings of rule_ids
*/
export const getSimpleRuleAsNdjson = (ruleIds: string[]): Buffer => {
export const getSimpleRuleAsNdjson = (ruleIds: string[], enabled = false): Buffer => {
const stringOfRules = ruleIds.map((ruleId) => {
const simpleRule = getSimpleRule(ruleId);
const simpleRule = getSimpleRule(ruleId, enabled);
return JSON.stringify(simpleRule);
});
return Buffer.from(stringOfRules.join('\n'));
Expand Down