Skip to content

Commit

Permalink
[ESO] Add flag to allow ESO consumers to opt-out of highly random UIDs (
Browse files Browse the repository at this point in the history
elastic#198287)

Closes elastic#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 <[email protected]>
Co-authored-by: Jeramy Soucy <[email protected]>
  • Loading branch information
3 people authored Nov 5, 2024
1 parent 3a26b5f commit 56c0806
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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
]);
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const createEncryptionExtension = (): jest.Mocked<ISavedObjectsEncryptionExtensi
isEncryptableType: jest.fn(),
decryptOrStripResponseAttributes: jest.fn(),
encryptAttributes: jest.fn(),
shouldEnforceRandomId: jest.fn(),
});

const createSecurityExtension = (): jest.Mocked<ISavedObjectsSecurityExtension> => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const createEncryptionExtension = (): jest.Mocked<ISavedObjectsEncryptionExtensi
isEncryptableType: jest.fn(),
decryptOrStripResponseAttributes: jest.fn(),
encryptAttributes: jest.fn(),
shouldEnforceRandomId: jest.fn(),
});

const createSecurityExtension = (): jest.Mocked<ISavedObjectsSecurityExtension> => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export class EncryptedSavedObjectAttributesDefinition {
public readonly attributesToEncrypt: ReadonlySet<string>;
private readonly attributesToIncludeInAAD: ReadonlySet<string> | undefined;
private readonly attributesToStrip: ReadonlySet<string>;
public readonly enforceRandomId: boolean;

constructor(typeRegistration: EncryptedSavedObjectTypeRegistration) {
if (typeRegistration.attributesToIncludeInAAD) {
Expand Down Expand Up @@ -49,6 +50,8 @@ export class EncryptedSavedObjectAttributesDefinition {
}
}

this.enforceRandomId = typeRegistration.enforceRandomId !== false;

this.attributesToEncrypt = attributesToEncrypt;
this.attributesToStrip = attributesToStrip;
this.attributesToIncludeInAAD = typeRegistration.attributesToIncludeInAAD;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export interface EncryptedSavedObjectTypeRegistration {
readonly type: string;
readonly attributesToEncrypt: ReadonlySet<string | AttributeToEncrypt>;
readonly attributesToIncludeInAAD?: ReadonlySet<string>;
readonly enforceRandomId?: boolean;
}

/**
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ export class SavedObjectsEncryptionExtension implements ISavedObjectsEncryptionE
return this._service.isRegistered(type);
}

shouldEnforceRandomId(type: string) {
return this._service.shouldEnforceRandomId(type);
}

async decryptOrStripResponseAttributes<T, R extends SavedObject<T>>(
response: R,
originalAttributes?: T
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -83,6 +85,30 @@ export const plugin: PluginInitializer<void, void, PluginsSetup, PluginsStart> =
});
}

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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.'
);
});
});
});
});
});
}

0 comments on commit 56c0806

Please sign in to comment.