From 56c0806af5a7f20903e92bfe88dc227e93ca2858 Mon Sep 17 00:00:00 2001 From: Sid Date: Tue, 5 Nov 2024 15:40:53 +0100 Subject: [PATCH] [ESO] Add flag to allow ESO consumers to opt-out of highly random UIDs (#198287) Closes https://github.com/elastic/kibana/issues/194692 ## Summary Allow consumers of ESOs to explicitly opt out of the strict highly random UID requirements while registering the ESO type ### Description The `getValidId` method was updated to allow consumers of Encrypted Saved Objects to explicitly opt-out of the enforced random ID requirement. This change is added during ESO registration - consumers can now pass a new field to opt-out of random UIDs. Additional changes - Updated canSpecifyID logic: - The canSpecifyID condition now also checks if enforceRandomId is explicitly set to false. This opt-out approach allows specific ESOs to bypass the random ID enforcement without affecting the default behavior, keeping it secure by default. During the registration phase of the saved object, consumers can now specify if they'd like to opt-out of the random ID ``` savedObjects.registerType({ name: TYPE_WITH_PREDICTABLE_ID, //... }); encryptedSavedObjects.registerType({ type: TYPE_WITH_PREDICTABLE_ID, //... enforceRandomId: false, }); ``` ### Release notes Improves Encrypted Saved Objects (ESO) ID validation by adding an enforceRandomId parameter, allowing consumers to opt out of the default random ID requirement for specific use cases. ### Checklist - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Elastic Machine Co-authored-by: Jeramy Soucy --- .../src/lib/apis/helpers/common.ts | 12 +- .../repository.encryption_extension.test.ts | 40 ++++++ .../mocks/saved_objects_extensions.mock.ts | 1 + .../src/saved_objects_extensions.mock.ts | 1 + .../src/extensions/encryption.ts | 8 ++ .../encrypted_saved_object_type_definition.ts | 3 + .../encrypted_saved_objects_service.test.ts | 21 +++ .../crypto/encrypted_saved_objects_service.ts | 11 ++ .../saved_objects_encryption_extension.ts | 4 + .../api_consumer_plugin/server/index.ts | 26 ++++ .../tests/encrypted_saved_objects_api.ts | 126 ++++++++++++++++++ 11 files changed, 249 insertions(+), 4 deletions(-) diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/helpers/common.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/helpers/common.ts index 27bd0918b0f9b..870f6833b4edc 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/helpers/common.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/apis/helpers/common.ts @@ -105,10 +105,14 @@ export class CommonHelper { if (!id) { return SavedObjectsUtils.generateId(); } - // only allow a specified ID if we're overwriting an existing ESO with a Version - // this helps us ensure that the document really was previously created using ESO - // and not being used to get around the specified ID limitation - const canSpecifyID = (overwrite && version) || SavedObjectsUtils.isRandomId(id); + + const shouldEnforceRandomId = this.encryptionExtension?.shouldEnforceRandomId(type); + + // Allow specified ID if: + // 1. we're overwriting an existing ESO with a Version (this helps us ensure that the document really was previously created using ESO) + // 2. enforceRandomId is explicitly set to false + const canSpecifyID = + !shouldEnforceRandomId || (overwrite && version) || SavedObjectsUtils.isRandomId(id); if (!canSpecifyID) { throw SavedObjectsErrorHelpers.createBadRequestError( 'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.' diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.encryption_extension.test.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.encryption_extension.test.ts index f5c8c8518a58a..cf66621565577 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.encryption_extension.test.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/repository.encryption_extension.test.ts @@ -261,6 +261,7 @@ describe('SavedObjectsRepository Encryption Extension', () => { it(`fails if non-UUID ID is specified for encrypted type`, async () => { mockEncryptionExt.isEncryptableType.mockReturnValue(true); + mockEncryptionExt.shouldEnforceRandomId.mockReturnValue(true); mockEncryptionExt.decryptOrStripResponseAttributes.mockResolvedValue({ ...encryptedSO, ...decryptedStrippedAttributes, @@ -291,6 +292,25 @@ describe('SavedObjectsRepository Encryption Extension', () => { ).resolves.not.toThrowError(); }); + it('allows to opt-out of random ID enforcement', async () => { + mockEncryptionExt.isEncryptableType.mockReturnValue(true); + mockEncryptionExt.shouldEnforceRandomId.mockReturnValue(false); + mockEncryptionExt.decryptOrStripResponseAttributes.mockResolvedValue({ + ...encryptedSO, + ...decryptedStrippedAttributes, + }); + + const result = await repository.create(encryptedSO.type, encryptedSO.attributes, { + id: encryptedSO.id, + version: mockVersion, + }); + + expect(client.create).toHaveBeenCalled(); + expect(mockEncryptionExt.isEncryptableType).toHaveBeenCalledWith(encryptedSO.type); + expect(mockEncryptionExt.shouldEnforceRandomId).toHaveBeenCalledWith(encryptedSO.type); + expect(result.id).toBe(encryptedSO.id); + }); + describe('namespace', () => { const doTest = async (optNamespace: string, expectNamespaceInDescriptor: boolean) => { const options = { overwrite: true, namespace: optNamespace }; @@ -483,6 +503,7 @@ describe('SavedObjectsRepository Encryption Extension', () => { it(`fails if non-UUID ID is specified for encrypted type`, async () => { mockEncryptionExt.isEncryptableType.mockReturnValue(true); + mockEncryptionExt.shouldEnforceRandomId.mockReturnValue(true); const result = await bulkCreateSuccess(client, repository, [ encryptedSO, // Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID ]); @@ -529,6 +550,25 @@ describe('SavedObjectsRepository Encryption Extension', () => { expect(result.saved_objects.length).toBe(1); expect(result.saved_objects[0].error).toBeUndefined(); }); + + it('allows to opt-out of random ID enforcement', async () => { + mockEncryptionExt.isEncryptableType.mockReturnValue(true); + mockEncryptionExt.shouldEnforceRandomId.mockReturnValue(false); + mockEncryptionExt.decryptOrStripResponseAttributes.mockResolvedValue({ + ...encryptedSO, + ...decryptedStrippedAttributes, + }); + + const result = await bulkCreateSuccess(client, repository, [ + { ...encryptedSO, version: mockVersion }, + ]); + + expect(client.bulk).toHaveBeenCalled(); + expect(result.saved_objects).not.toBeUndefined(); + expect(result.saved_objects.length).toBe(1); + expect(result.saved_objects[0].error).toBeUndefined(); + expect(result.saved_objects[0].id).toBe(encryptedSO.id); + }); }); describe('#bulkUpdate', () => { diff --git a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/mocks/saved_objects_extensions.mock.ts b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/mocks/saved_objects_extensions.mock.ts index 2061bb63240b2..9dc7c0f0133c5 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-internal/src/mocks/saved_objects_extensions.mock.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-internal/src/mocks/saved_objects_extensions.mock.ts @@ -17,6 +17,7 @@ const createEncryptionExtension = (): jest.Mocked => ({ diff --git a/packages/core/saved-objects/core-saved-objects-api-server-mocks/src/saved_objects_extensions.mock.ts b/packages/core/saved-objects/core-saved-objects-api-server-mocks/src/saved_objects_extensions.mock.ts index 2a2d121b568be..776ecfe3a7385 100644 --- a/packages/core/saved-objects/core-saved-objects-api-server-mocks/src/saved_objects_extensions.mock.ts +++ b/packages/core/saved-objects/core-saved-objects-api-server-mocks/src/saved_objects_extensions.mock.ts @@ -18,6 +18,7 @@ const createEncryptionExtension = (): jest.Mocked => ({ diff --git a/packages/core/saved-objects/core-saved-objects-server/src/extensions/encryption.ts b/packages/core/saved-objects/core-saved-objects-server/src/extensions/encryption.ts index 3fdb29203fe13..4560ab5672666 100644 --- a/packages/core/saved-objects/core-saved-objects-server/src/extensions/encryption.ts +++ b/packages/core/saved-objects/core-saved-objects-server/src/extensions/encryption.ts @@ -39,6 +39,14 @@ export interface ISavedObjectsEncryptionExtension { */ isEncryptableType: (type: string) => boolean; + /** + * Returns false if ESO type explicitly opts out of highly random UID + * + * @param type the string name of the object type + * @returns boolean, true by default unless explicitly set to false + */ + shouldEnforceRandomId: (type: string) => boolean; + /** * Given a saved object, will return a decrypted saved object or will strip * attributes from the returned object if decryption fails. diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts index d8ce2daa6efbe..bb07842e2bab5 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts @@ -16,6 +16,7 @@ export class EncryptedSavedObjectAttributesDefinition { public readonly attributesToEncrypt: ReadonlySet; private readonly attributesToIncludeInAAD: ReadonlySet | undefined; private readonly attributesToStrip: ReadonlySet; + public readonly enforceRandomId: boolean; constructor(typeRegistration: EncryptedSavedObjectTypeRegistration) { if (typeRegistration.attributesToIncludeInAAD) { @@ -49,6 +50,8 @@ export class EncryptedSavedObjectAttributesDefinition { } } + this.enforceRandomId = typeRegistration.enforceRandomId !== false; + this.attributesToEncrypt = attributesToEncrypt; this.attributesToStrip = attributesToStrip; this.attributesToIncludeInAAD = typeRegistration.attributesToIncludeInAAD; diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts index 1691d3f4c0610..67c972ec5f859 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts @@ -2405,3 +2405,24 @@ describe('#decryptAttributesSync', () => { }); }); }); + +describe('#shouldEnforceRandomId', () => { + it('defaults to true if enforceRandomId is undefined', () => { + service.registerType({ type: 'known-type-1', attributesToEncrypt: new Set(['attr']) }); + expect(service.shouldEnforceRandomId('known-type-1')).toBe(true); + }); + it('should return the value of enforceRandomId if it is defined', () => { + service.registerType({ + type: 'known-type-1', + attributesToEncrypt: new Set(['attr']), + enforceRandomId: false, + }); + service.registerType({ + type: 'known-type-2', + attributesToEncrypt: new Set(['attr']), + enforceRandomId: true, + }); + expect(service.shouldEnforceRandomId('known-type-1')).toBe(false); + expect(service.shouldEnforceRandomId('known-type-2')).toBe(true); + }); +}); diff --git a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts index 44072a0828d48..d2c7d9975a9ca 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts @@ -33,6 +33,7 @@ export interface EncryptedSavedObjectTypeRegistration { readonly type: string; readonly attributesToEncrypt: ReadonlySet; readonly attributesToIncludeInAAD?: ReadonlySet; + readonly enforceRandomId?: boolean; } /** @@ -152,6 +153,16 @@ export class EncryptedSavedObjectsService { return this.typeDefinitions.has(type); } + /** + * Checks whether the ESO type has explicitly opted out of enforcing random IDs. + * @param type Saved object type. + * @returns boolean - true unless explicitly opted out by setting enforceRandomId to false + */ + public shouldEnforceRandomId(type: string) { + const typeDefinition = this.typeDefinitions.get(type); + return typeDefinition?.enforceRandomId !== false; + } + /** * Takes saved object attributes for the specified type and, depending on the type definition, * either decrypts or strips encrypted attributes (e.g. in case AAD or encryption key has changed diff --git a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/saved_objects_encryption_extension.ts b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/saved_objects_encryption_extension.ts index 01c35c7403fdf..45e0f6a46c892 100644 --- a/x-pack/plugins/encrypted_saved_objects/server/saved_objects/saved_objects_encryption_extension.ts +++ b/x-pack/plugins/encrypted_saved_objects/server/saved_objects/saved_objects_encryption_extension.ts @@ -40,6 +40,10 @@ export class SavedObjectsEncryptionExtension implements ISavedObjectsEncryptionE return this._service.isRegistered(type); } + shouldEnforceRandomId(type: string) { + return this._service.shouldEnforceRandomId(type); + } + async decryptOrStripResponseAttributes>( response: R, originalAttributes?: T diff --git a/x-pack/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts b/x-pack/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts index c7946b2e68131..6944123790157 100644 --- a/x-pack/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts +++ b/x-pack/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin/server/index.ts @@ -32,6 +32,8 @@ const SAVED_OBJECT_WITH_MIGRATION_TYPE = 'saved-object-with-migration'; const SAVED_OBJECT_MV_TYPE = 'saved-object-mv'; +const TYPE_WITH_PREDICTABLE_ID = 'type-with-predictable-ids'; + interface MigratedTypePre790 { nonEncryptedAttribute: string; encryptedAttribute: string; @@ -83,6 +85,30 @@ export const plugin: PluginInitializer = }); } + core.savedObjects.registerType({ + name: TYPE_WITH_PREDICTABLE_ID, + hidden: false, + namespaceType: 'single', + mappings: deepFreeze({ + properties: { + publicProperty: { type: 'keyword' }, + publicPropertyExcludedFromAAD: { type: 'keyword' }, + publicPropertyStoredEncrypted: { type: 'binary' }, + privateProperty: { type: 'binary' }, + }, + }), + }); + + deps.encryptedSavedObjects.registerType({ + type: TYPE_WITH_PREDICTABLE_ID, + attributesToEncrypt: new Set([ + 'privateProperty', + { key: 'publicPropertyStoredEncrypted', dangerouslyExposeValue: true }, + ]), + attributesToIncludeInAAD: new Set(['publicProperty']), + enforceRandomId: false, + }); + core.savedObjects.registerType({ name: SAVED_OBJECT_WITHOUT_SECRET_TYPE, hidden: false, diff --git a/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts b/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts index 4687a01858260..23aa9017e52ea 100644 --- a/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts +++ b/x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts @@ -26,6 +26,8 @@ export default function ({ getService }: FtrProviderContext) { 'saved-object-with-secret-and-multiple-spaces'; const SAVED_OBJECT_WITHOUT_SECRET_TYPE = 'saved-object-without-secret'; + const TYPE_WITH_PREDICTABLE_ID = 'type-with-predictable-ids'; + function runTests( encryptedSavedObjectType: string, getURLAPIBaseURL: () => string, @@ -900,5 +902,129 @@ export default function ({ getService }: FtrProviderContext) { } }); }); + + describe('enforceRandomId', () => { + describe('false', () => { + it('#create allows setting non-random ID', async () => { + const id = 'my_predictable_id'; + + const savedObjectOriginalAttributes = { + publicProperty: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + }; + + const { body: response } = await supertest + .post(`/api/saved_objects/${TYPE_WITH_PREDICTABLE_ID}/${id}`) + .set('kbn-xsrf', 'xxx') + .send({ attributes: savedObjectOriginalAttributes }) + .expect(200); + + expect(response.id).to.be(id); + }); + + it('#bulkCreate not enforcing random ID allows to specify ID', async () => { + const bulkCreateParams = [ + { + type: TYPE_WITH_PREDICTABLE_ID, + id: 'my_predictable_id', + attributes: { + publicProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + }, + }, + { + type: TYPE_WITH_PREDICTABLE_ID, + id: 'my_predictable_id_2', + attributes: { + publicProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + }, + }, + ]; + + const { + body: { saved_objects: savedObjects }, + } = await supertest + .post('/api/saved_objects/_bulk_create') + .set('kbn-xsrf', 'xxx') + .send(bulkCreateParams) + .expect(200); + + expect(savedObjects).to.have.length(bulkCreateParams.length); + expect(savedObjects[0].id).to.be('my_predictable_id'); + expect(savedObjects[1].id).to.be('my_predictable_id_2'); + }); + }); + + describe('true or undefined', () => { + it('#create setting a predictable id on ESO types that have not opted out throws an error', async () => { + const id = 'my_predictable_id'; + + const savedObjectOriginalAttributes = { + publicProperty: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + }; + + const { body: response } = await supertest + .post(`/api/saved_objects/saved-object-with-secret/${id}`) + .set('kbn-xsrf', 'xxx') + .send({ attributes: savedObjectOriginalAttributes }) + .expect(400); + + expect(response.message).to.contain( + 'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.' + ); + }); + + it('#bulkCreate setting random ID on ESO types that have not opted out throws an error', async () => { + const bulkCreateParams = [ + { + type: SAVED_OBJECT_WITH_SECRET_TYPE, + id: 'my_predictable_id', + attributes: { + publicProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + }, + }, + { + type: SAVED_OBJECT_WITH_SECRET_TYPE, + id: 'my_predictable_id_2', + attributes: { + publicProperty: randomness.string(), + publicPropertyExcludedFromAAD: randomness.string(), + publicPropertyStoredEncrypted: randomness.string(), + privateProperty: randomness.string(), + }, + }, + ]; + + const { + body: { saved_objects: savedObjects }, + } = await supertest + .post('/api/saved_objects/_bulk_create') + .set('kbn-xsrf', 'xxx') + .send(bulkCreateParams) + .expect(200); + + expect(savedObjects).to.have.length(bulkCreateParams.length); + + savedObjects.forEach((savedObject: any) => { + expect(savedObject.error.message).to.contain( + 'Predefined IDs are not allowed for saved objects with encrypted attributes unless the ID is a UUID.' + ); + }); + }); + }); + }); }); }