From 3e7423a0e9fdc6cb8c96462ed516f4cca80d6c8e Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Tue, 17 Aug 2021 13:59:25 -0600 Subject: [PATCH] [Security Solutions][Detection Engine] Migrates exception lists to saved object references (Part 2) (#108291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This is part 2 to addressing the issue seen here: #101975 (Part 1 #107064) This adds the alerting migration scripts and unit tests for exception list containers on Kibana startup for `7.15.0` We only migrate if we find these conditions and cases: - `exceptionLists` are an `array` and not `null`, `undefined`, or malformed data. - The exceptionList item is an `object` and its `id` is a `string` and not `null`, `undefined`, or malformed data - The existing references do not already have an exceptionItem reference already found within it. We migrate on the common use case - The saved object references do not exist but we have exceptionList items with the id's to create the saved object references, 👍 so we migrate. - The alert contains no exception list items, in which case we have nothing to migrate We do these additional (mis-use) cases and steps as well. These should _NOT_ be common things that happen but we safe guard for them here: - If the migration is run twice we are idempotent and do _NOT_ add duplicates list items or remove items. - If the migration was partially successful but re-run a second time, we only add what is missing. Again no duplicates or removed items should occur. - If the `exceptionLists` contains invalid data shape or not enough information to migrate, we filter it out and ignore it - If the saved object references already exists and contains a different or foreign value, we will retain the foreign reference(s) and still migrate. ## Manual testing There are unit tests but for any manual testing or verification you can do the following: Create a few alerts through the `security_solution` application with exception lists Screen Shot 2021-08-11 at 5 42 31 PM Use the dev tools to de-migrate as well as to test end to end like so: ```json # First get an "_id" with an exceptions list like below. Mine I found was: "alert:38482620-ef1b-11eb-ad71-7de7959be71c": GET .kibana/_search { "query": { "terms": { "alert.alertTypeId": [ "siem.signals" ] } }, "size": 10000 } ``` With Kibana running downgrade and remove the references as a test: ```json # Set saved object array references as empty arrays and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [] """, "lang": "painless" } } # Double check the references is empty and the version is 7.14.0 GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Reload the alert in the `security_solution` and notice you get these errors until you restart Kibana to cause a migration moving forward ```sh server log [17:35:16.914] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.914] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.915] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"endpoint_list","namespace_type":"agnostic","id":"endpoint_list","type":"endpoint"} server log [17:35:16.940] [error][plugins][securitySolution] Cannot get a saved object reference using an index which is larger than the saved object references. Index is:1 which is larger than the savedObjectReferences:[] server log [17:35:16.940] [error][plugins][securitySolution] The saved object references were not found for our exception list when we were expecting to find it. Kibana migrations might not have run correctly or someone might have removed the saved object references manually. Returning the last known good exception list id which might not work. exceptionItem with its id being returned is: {"list_id":"cd152d0d-3590-4a45-a478-eed04da7936b","namespace_type":"single","id":"50e3bd70-ef1b-11eb-ad71-7de7959be71c","type":"detection"} ``` Restart Kibana and you should no longer have errors in the Kibana console. If you do this query in dev tools ```json # Check that the `migrationVersion` is `7.15.0` and that we have a `references` array filled out with the correct structure GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` You should notice that you now have a `references` array filled out: ```json "references" : [ { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ], "migrationVersion" : { "alert" : "7.15.0" } ``` For testing [idempotentence](https://en.wikipedia.org/wiki/Idempotence) Run just this to downgrade and restart Kibana and you should notice on a GET that we do not have anything extra in the references array: ```json # Set our migration version to be 7.14.0 only POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; """, "lang": "painless" } } # Double check the `references` is still there, and we do not get errors or changes to `references` after we restart Kibana GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` For testing foreign keys: ```json # Set saved object array references to foreign keys and set our migration version to be 7.14.0 POST .kibana/_update/alert:38482620-ef1b-11eb-ad71-7de7959be71c { "script" : { "source": """ ctx._source.migrationVersion.alert = "7.14.0"; ctx._source.references = [["name" : "foreign", "id" : "123", "type" : "some-type"]]; """, "lang": "painless" } } ``` Restart, ensure no errors in Kibana console and do a get call to ensure we have the foreign mixed with valid values: ```json GET .kibana/_doc/alert:38482620-ef1b-11eb-ad71-7de7959be71c ``` Should return this data: ```json "type" : "alert", "references" : [ { "name" : "foreign", "id" : "123", "type" : "some-type" }, { "name" : "param:exceptionsList_0", "id" : "endpoint_list", "type" : "exception-list-agnostic" }, { "name" : "param:exceptionsList_1", "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", "type" : "exception-list" } ] ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../server/saved_objects/migrations.test.ts | 404 ++++++++++++++++++ .../server/saved_objects/migrations.ts | 100 +++++ .../spaces_only/tests/alerting/migrations.ts | 22 + .../functional/es_archives/alerts/data.json | 54 +++ 4 files changed, 580 insertions(+) diff --git a/x-pack/plugins/alerting/server/saved_objects/migrations.test.ts b/x-pack/plugins/alerting/server/saved_objects/migrations.test.ts index 512f4618792fb..b1460a5fe5cd8 100644 --- a/x-pack/plugins/alerting/server/saved_objects/migrations.test.ts +++ b/x-pack/plugins/alerting/server/saved_objects/migrations.test.ts @@ -1012,6 +1012,410 @@ describe('successful migrations', () => { }); }); }); + + describe('7.15.0', () => { + test('security solution is migrated to saved object references if it has 1 exceptionsList', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', // extra data to ensure we do not overwrite other params + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }); + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution is migrated to saved object references if it has 2 exceptionsLists', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', // extra data to ensure we do not overwrite other params + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'agnostic', + }, + { + id: '789', + list_id: '0123', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }); + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list-agnostic', + }, + { + name: 'param:exceptionsList_1', + id: '789', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution is migrated to saved object references if it has 3 exceptionsLists', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', // extra data to ensure we do not overwrite other params + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + { + id: '789', + list_id: '0123', + type: 'detection', + namespace_type: 'agnostic', + }, + { + id: '101112', + list_id: '777', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }); + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_1', + id: '789', + type: 'exception-list-agnostic', + }, + { + name: 'param:exceptionsList_2', + id: '101112', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution does not change anything if exceptionsList is missing', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + }, + }); + + expect(migration7150(alert, migrationContext)).toEqual(alert); + }); + + test('security solution will keep existing references if we do not have an exceptionsList but we do already have references', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + }, + }), + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }; + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution keep any foreign references if they exist but still migrate other references', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + { + id: '789', + list_id: '0123', + type: 'detection', + namespace_type: 'single', + }, + { + id: '101112', + list_id: '777', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }), + references: [ + { + name: 'foreign-name', + id: '999', + type: 'foreign-name', + }, + ], + }; + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'foreign-name', + id: '999', + type: 'foreign-name', + }, + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_1', + id: '789', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_2', + id: '101112', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution is idempotent and if re-run on the same migrated data will keep the same items', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + { + id: '789', + list_id: '0123', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }), + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_1', + id: '789', + type: 'exception-list', + }, + ], + }; + + expect(migration7150(alert, migrationContext)).toEqual(alert); + }); + + test('security solution will migrate with only missing data if we have partially migrated data', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + { + id: '789', + list_id: '0123', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }), + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }; + + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_1', + id: '789', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution will not migrate if exception list if it is invalid data', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [{ invalid: 'invalid' }], + }, + }), + }; + expect(migration7150(alert, migrationContext)).toEqual(alert); + }); + + test('security solution will migrate valid data if it is mixed with invalid data', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [ + { + id: '123', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + { id: 555 }, // <-- Id is a number and not a string, and is invalid + { + id: '456', + list_id: '456', + type: 'detection', + namespace_type: 'single', + }, + ], + }, + }), + }; + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + { + name: 'param:exceptionsList_1', + id: '456', + type: 'exception-list', + }, + ], + }); + }); + + test('security solution will not migrate if exception list is invalid data but will keep existing references', () => { + const migration7150 = getMigrations(encryptedSavedObjectsSetup)['7.15.0']; + const alert = { + ...getMockData({ + alertTypeId: 'siem.signals', + params: { + note: 'some note', + exceptionsList: [{ invalid: 'invalid' }], + }, + }), + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }; + expect(migration7150(alert, migrationContext)).toEqual({ + ...alert, + references: [ + { + name: 'param:exceptionsList_0', + id: '123', + type: 'exception-list', + }, + ], + }); + }); + }); }); describe('handles errors during migrations', () => { diff --git a/x-pack/plugins/alerting/server/saved_objects/migrations.ts b/x-pack/plugins/alerting/server/saved_objects/migrations.ts index 944acbdca0182..6823a9b9b20da 100644 --- a/x-pack/plugins/alerting/server/saved_objects/migrations.ts +++ b/x-pack/plugins/alerting/server/saved_objects/migrations.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { isString } from 'lodash/fp'; import { LogMeta, SavedObjectMigrationMap, @@ -13,6 +14,7 @@ import { SavedObjectMigrationContext, SavedObjectAttributes, SavedObjectAttribute, + SavedObjectReference, } from '../../../../../src/core/server'; import { RawAlert, RawAlertAction } from '../types'; import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server'; @@ -91,12 +93,19 @@ export function getMigrations( pipeMigrations(removeNullAuthorFromSecurityRules) ); + const migrationSecurityRules715 = createEsoMigration( + encryptedSavedObjects, + (doc): doc is SavedObjectUnsanitizedDoc => isSecuritySolutionRule(doc), + pipeMigrations(addExceptionListsToReferences) + ); + return { '7.10.0': executeMigrationWithErrorHandling(migrationWhenRBACWasIntroduced, '7.10.0'), '7.11.0': executeMigrationWithErrorHandling(migrationAlertUpdatedAtAndNotifyWhen, '7.11.0'), '7.11.2': executeMigrationWithErrorHandling(migrationActions7112, '7.11.2'), '7.13.0': executeMigrationWithErrorHandling(migrationSecurityRules713, '7.13.0'), '7.14.1': executeMigrationWithErrorHandling(migrationSecurityRules714, '7.14.1'), + '7.15.0': executeMigrationWithErrorHandling(migrationSecurityRules715, '7.15.0'), }; } @@ -467,6 +476,97 @@ function removeNullAuthorFromSecurityRules( }; } +/** + * This migrates exception list containers to saved object references on an upgrade. + * We only migrate if we find these conditions: + * - exceptionLists are an array and not null, undefined, or malformed data. + * - The exceptionList item is an object and id is a string and not null, undefined, or malformed data + * - The existing references do not already have an exceptionItem reference already found within it. + * Some of these issues could crop up during either user manual errors of modifying things, earlier migration + * issues, etc... + * @param doc The document that might have exceptionListItems to migrate + * @returns The document migrated with saved object references + */ +function addExceptionListsToReferences( + doc: SavedObjectUnsanitizedDoc +): SavedObjectUnsanitizedDoc { + const { + attributes: { + params: { exceptionsList }, + }, + references, + } = doc; + if (!Array.isArray(exceptionsList)) { + // early return if we are not an array such as being undefined or null or malformed. + return doc; + } else { + const exceptionsToTransform = removeMalformedExceptionsList(exceptionsList); + const newReferences = exceptionsToTransform.flatMap( + (exceptionItem, index) => { + const existingReferenceFound = references?.find((reference) => { + return ( + reference.id === exceptionItem.id && + ((reference.type === 'exception-list' && exceptionItem.namespace_type === 'single') || + (reference.type === 'exception-list-agnostic' && + exceptionItem.namespace_type === 'agnostic')) + ); + }); + if (existingReferenceFound) { + // skip if the reference already exists for some uncommon reason so we do not add an additional one. + // This enables us to be idempotent and you can run this migration multiple times and get the same output. + return []; + } else { + return [ + { + name: `param:exceptionsList_${index}`, + id: String(exceptionItem.id), + type: + exceptionItem.namespace_type === 'agnostic' + ? 'exception-list-agnostic' + : 'exception-list', + }, + ]; + } + } + ); + if (references == null && newReferences.length === 0) { + // Avoid adding an empty references array if the existing saved object never had one to begin with + return doc; + } else { + return { ...doc, references: [...(references ?? []), ...newReferences] }; + } + } +} + +/** + * This will do a flatMap reduce where we only return exceptionsLists and their items if: + * - exceptionLists are an array and not null, undefined, or malformed data. + * - The exceptionList item is an object and id is a string and not null, undefined, or malformed data + * + * Some of these issues could crop up during either user manual errors of modifying things, earlier migration + * issues, etc... + * @param exceptionsList The list of exceptions + * @returns The exception lists if they are a valid enough shape + */ +function removeMalformedExceptionsList( + exceptionsList: SavedObjectAttribute +): SavedObjectAttributes[] { + if (!Array.isArray(exceptionsList)) { + // early return if we are not an array such as being undefined or null or malformed. + return []; + } else { + return exceptionsList.flatMap((exceptionItem) => { + if (!(exceptionItem instanceof Object) || !isString(exceptionItem.id)) { + // return early if we are not an object such as being undefined or null or malformed + // or the exceptionItem.id is not a string from being malformed + return []; + } else { + return [exceptionItem]; + } + }); + } +} + function pipeMigrations(...migrations: AlertMigration[]): AlertMigration { return (doc: SavedObjectUnsanitizedDoc) => migrations.reduce((migratedDoc, nextMigration) => nextMigration(migratedDoc), doc); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts index 0eaeaf9a4b7e7..81b544ac97152 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/migrations.ts @@ -11,6 +11,7 @@ import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export export default function createGetTests({ getService }: FtrProviderContext) { + const es = getService('es'); const supertest = getService('supertest'); const esArchiver = getService('esArchiver'); @@ -175,5 +176,26 @@ export default function createGetTests({ getService }: FtrProviderContext) { }, ]); }); + + it('7.15.0 migrates security_solution alerts with exceptionLists to be saved object references', async () => { + // NOTE: We hae to use elastic search directly against the ".kibana" index because alerts do not expose the references which we want to test exists + const response = await es.get<{ references: [{}] }>({ + index: '.kibana', + id: 'alert:38482620-ef1b-11eb-ad71-7de7959be71c', + }); + expect(response.statusCode).to.eql(200); + expect(response.body._source?.references).to.eql([ + { + name: 'param:exceptionsList_0', + id: 'endpoint_list', + type: 'exception-list-agnostic', + }, + { + name: 'param:exceptionsList_1', + id: '50e3bd70-ef1b-11eb-ad71-7de7959be71c', + type: 'exception-list', + }, + ]); + }); }); } diff --git a/x-pack/test/functional/es_archives/alerts/data.json b/x-pack/test/functional/es_archives/alerts/data.json index 26f201c095dca..2ce6be7b4816c 100644 --- a/x-pack/test/functional/es_archives/alerts/data.json +++ b/x-pack/test/functional/es_archives/alerts/data.json @@ -333,3 +333,57 @@ } } } + +{ + "type": "doc", + "value": { + "id": "alert:38482620-ef1b-11eb-ad71-7de7959be71c", + "index": ".kibana_1", + "source": { + "alert" : { + "name" : "test upgrade of exceptionsList", + "alertTypeId" : "siem.signals", + "consumer" : "alertsFixture", + "params" : { + "ruleId" : "4ec223b9-77fa-4895-8539-6b3e586a2858", + "exceptionsList" : [ + { + "id" : "endpoint_list", + "list_id" : "endpoint_list", + "namespace_type" : "agnostic", + "type" : "endpoint" + }, + { + "list_id" : "cd152d0d-3590-4a45-a478-eed04da7936b", + "namespace_type" : "single", + "id" : "50e3bd70-ef1b-11eb-ad71-7de7959be71c", + "type" : "detection" + } + ] + }, + "schedule" : { + "interval" : "1m" + }, + "enabled" : true, + "actions" : [ ], + "throttle" : null, + "apiKeyOwner" : null, + "apiKey" : null, + "createdBy" : "elastic", + "updatedBy" : "elastic", + "createdAt" : "2021-07-27T20:42:55.896Z", + "muteAll" : false, + "mutedInstanceIds" : [ ], + "scheduledTaskId" : null, + "tags": [] + }, + "type" : "alert", + "migrationVersion" : { + "alert" : "7.8.0" + }, + "updated_at" : "2021-08-13T23:00:11.985Z", + "references": [ + ] + } + } +}