From 8fe426521e04c1652370c06e4862044567b75ebb Mon Sep 17 00:00:00 2001 From: "Devin W. Hurley" Date: Thu, 17 Sep 2020 18:40:07 -0400 Subject: [PATCH] [7.9] [Security Solution] [Detections] Remove file validation on import route (#77770) (#77822) * utlize schema.any() for validation on file in body of import rules request, adds new functional tests and unit tests to make sure we can reach and don't go past bounds. These tests would have helped uncover performance issues io-ts gave us with validating the import rules file object * fix type check failure * updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled param defaulted to false * updates comments in e2e tests for import rules route * fix tests after adding enabled boolean in test utils --- .../routes/rules/import_rules_route.test.ts | 25 ++++++++ .../routes/rules/import_rules_route.ts | 10 +-- .../basic/tests/import_rules.ts | 62 +++++++++++++++++-- .../security_and_spaces/tests/import_rules.ts | 12 ++-- .../detection_engine_api_integration/utils.ts | 8 ++- 5 files changed, 99 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts index f6a903ca9a4ee..0a922e2a94012 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts @@ -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); @@ -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) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index 18eea7c45585f..2a4dee410b29a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -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 { @@ -47,7 +46,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( @@ -57,10 +56,7 @@ export const importRulesRoute = (router: IRouter, config: ConfigType, ml: SetupP query: buildRouteValidation( importRulesQuerySchema ), - body: buildRouteValidation< - typeof importRulesPayloadSchema, - ImportRulesPayloadSchemaDecoded - >(importRulesPayloadSchema), + body: schema.any(), // validation on file object is accomplished later in the handler. }, options: { tags: ['access:securitySolution'], diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts index 108ca365bc14f..c6294cfe6ec28 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts @@ -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 @@ -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`) @@ -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'); @@ -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 diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index e0b60ae1fbeeb..664077d5a4fab 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -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 @@ -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'); @@ -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 diff --git a/x-pack/test/detection_engine_api_integration/utils.ts b/x-pack/test/detection_engine_api_integration/utils.ts index d801b38ba9f53..8e100d7a95b22 100644 --- a/x-pack/test/detection_engine_api_integration/utils.ts +++ b/x-pack/test/detection_engine_api_integration/utils.ts @@ -53,10 +53,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', @@ -355,9 +357,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'));