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.

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

[8.x] [ESO] Add flag to allow ESO consumers to opt-out of highly random UIDs (#198287) #198956

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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.'
);
});
});
});
});
});
}