maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Allow predefined ids for encrypted saved objects (#83482)

Browse source 
* Allow predefined ids for encrypted saved objects

* Fix mock

* fix tests

* Added suggestions from code review

* added jsdocs params

* Fixed jsdocs
This commit is contained in:
Thom Heymann 2020-11-23 14:53:35 +00:00 committed by GitHub
parent d51437e891
commit 7d929fe903
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 160 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.test.ts
View file

@ -113,3 +113,18 @@ it('correctly determines attribute properties', () => {
}
}
});
it('it correctly sets allowPredefinedID', () => {
const defaultTypeDefinition = new EncryptedSavedObjectAttributesDefinition({
type: 'so-type',
attributesToEncrypt: new Set(['attr#1', 'attr#2']),
});
expect(defaultTypeDefinition.allowPredefinedID).toBe(false);
const typeDefinitionWithPredefinedIDAllowed = new EncryptedSavedObjectAttributesDefinition({
type: 'so-type',
attributesToEncrypt: new Set(['attr#1', 'attr#2']),
allowPredefinedID: true,
});
expect(typeDefinitionWithPredefinedIDAllowed.allowPredefinedID).toBe(true);
});

2
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_object_type_definition.ts
View file

@ -15,6 +15,7 @@ export class EncryptedSavedObjectAttributesDefinition {
public readonly attributesToEncrypt: ReadonlySet<string>;
private readonly attributesToExcludeFromAAD: ReadonlySet<string> | undefined;
private readonly attributesToStrip: ReadonlySet<string>;
public readonly allowPredefinedID: boolean;
constructor(typeRegistration: EncryptedSavedObjectTypeRegistration) {
const attributesToEncrypt = new Set<string>();
@ -34,6 +35,7 @@ export class EncryptedSavedObjectAttributesDefinition {
this.attributesToEncrypt = attributesToEncrypt;
this.attributesToStrip = attributesToStrip;
this.attributesToExcludeFromAAD = typeRegistration.attributesToExcludeFromAAD;
this.allowPredefinedID = !!typeRegistration.allowPredefinedID;
}
/**

7
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.mocks.ts
View file

@ -13,6 +13,7 @@ import {
function createEncryptedSavedObjectsServiceMock() {
return ({
isRegistered: jest.fn(),
canSpecifyID: jest.fn(),
stripOrDecryptAttributes: jest.fn(),
encryptAttributes: jest.fn(),
decryptAttributes: jest.fn(),
@ -52,6 +53,12 @@ export const encryptedSavedObjectsServiceMock = {
mock.isRegistered.mockImplementation(
(type) => registrations.findIndex((r) => r.type === type) >= 0
);
mock.canSpecifyID.mockImplementation((type, version, overwrite) => {
const registration = registrations.find((r) => r.type === type);
return (
registration === undefined || registration.allowPredefinedID || !!(version && overwrite)
);
});
mock.encryptAttributes.mockImplementation(async (descriptor, attrs) =>
processAttributes(
descriptor,

39
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.test.ts
View file

@ -89,6 +89,45 @@ describe('#isRegistered', () => {
});
});
describe('#canSpecifyID', () => {
it('returns true for unknown types', () => {
expect(service.canSpecifyID('unknown-type')).toBe(true);
});
it('returns true for types registered setting allowPredefinedID to true', () => {
service.registerType({
type: 'known-type-1',
attributesToEncrypt: new Set(['attr-1']),
allowPredefinedID: true,
});
expect(service.canSpecifyID('known-type-1')).toBe(true);
});
it('returns true when overwriting a saved object with a version specified even when allowPredefinedID is not set', () => {
service.registerType({
type: 'known-type-1',
attributesToEncrypt: new Set(['attr-1']),
});
expect(service.canSpecifyID('known-type-1', '2', true)).toBe(true);
expect(service.canSpecifyID('known-type-1', '2', false)).toBe(false);
expect(service.canSpecifyID('known-type-1', undefined, true)).toBe(false);
});
it('returns false for types registered without setting allowPredefinedID', () => {
service.registerType({ type: 'known-type-1', attributesToEncrypt: new Set(['attr-1']) });
expect(service.canSpecifyID('known-type-1')).toBe(false);
});
it('returns false for types registered setting allowPredefinedID to false', () => {
service.registerType({
type: 'known-type-1',
attributesToEncrypt: new Set(['attr-1']),
allowPredefinedID: false,
});
expect(service.canSpecifyID('known-type-1')).toBe(false);
});
});
describe('#stripOrDecryptAttributes', () => {
it('does not strip attributes from unknown types', async () => {
const attributes = { attrOne: 'one', attrTwo: 'two', attrThree: 'three' };

20
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
View file

@ -31,6 +31,7 @@ export interface EncryptedSavedObjectTypeRegistration {
readonly type: string;
readonly attributesToEncrypt: ReadonlySet<string | AttributeToEncrypt>;
readonly attributesToExcludeFromAAD?: ReadonlySet<string>;
readonly allowPredefinedID?: boolean;
}
/**
@ -144,6 +145,25 @@ export class EncryptedSavedObjectsService {
return this.typeDefinitions.has(type);
}
/**
* Checks whether ID can be specified for the provided saved object.
*
* If the type isn't registered as an encrypted saved object, or when overwriting an existing
* saved object with a version specified, this will return "true".
*
* @param type Saved object type.
* @param version Saved object version number which changes on each successful write operation.
* Can be used in conjunction with `overwrite` for implementing optimistic concurrency
* control.
* @param overwrite Overwrite existing documents.
*/
public canSpecifyID(type: string, version?: string, overwrite?: boolean) {
const typeDefinition = this.typeDefinitions.get(type);
return (
typeDefinition === undefined || typeDefinition.allowPredefinedID || !!(version && overwrite)
);
}
/**
* 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

69
x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.test.ts
View file

@ -30,6 +30,11 @@ beforeEach(() => {
{ key: 'attrNotSoSecret', dangerouslyExposeValue: true },
]),
},
{
type: 'known-type-predefined-id',
attributesToEncrypt: new Set(['attrSecret']),
allowPredefinedID: true,
},
]);
wrapper = new EncryptedSavedObjectsClientWrapper({
@ -72,16 +77,36 @@ describe('#create', () => {
expect(mockBaseClient.create).toHaveBeenCalledWith('unknown-type', attributes, options);
});
it('fails if type is registered and ID is specified', async () => {
it('fails if type is registered without allowPredefinedID and ID is specified', async () => {
const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };
await expect(wrapper.create('known-type', attributes, { id: 'some-id' })).rejects.toThrowError(
'Predefined IDs are not allowed for saved objects with encrypted attributes.'
'Predefined IDs are not allowed for encrypted saved objects of type "known-type".'
);
expect(mockBaseClient.create).not.toHaveBeenCalled();
});
it('succeeds if type is registered with allowPredefinedID and ID is specified', async () => {
const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };
const mockedResponse = {
id: 'some-id',
type: 'known-type-predefined-id',
attributes: { attrOne: 'one', attrSecret: '*secret*', attrThree: 'three' },
references: [],
};
mockBaseClient.create.mockResolvedValue(mockedResponse);
await expect(
wrapper.create('known-type-predefined-id', attributes, { id: 'some-id' })
).resolves.toEqual({
...mockedResponse,
attributes: { attrOne: 'one', attrThree: 'three' },
});
expect(mockBaseClient.create).toHaveBeenCalled();
});
it('allows a specified ID when overwriting an existing object', async () => {
const attributes = {
attrOne: 'one',
@ -299,7 +324,7 @@ describe('#bulkCreate', () => {
);
});
it('fails if ID is specified for registered type', async () => {
it('fails if ID is specified for registered type without allowPredefinedID', async () => {
const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };
const bulkCreateParams = [
@ -308,12 +333,48 @@ describe('#bulkCreate', () => {
];
await expect(wrapper.bulkCreate(bulkCreateParams)).rejects.toThrowError(
'Predefined IDs are not allowed for saved objects with encrypted attributes.'
'Predefined IDs are not allowed for encrypted saved objects of type "known-type".'
);
expect(mockBaseClient.bulkCreate).not.toHaveBeenCalled();
});
it('succeeds if ID is specified for registered type with allowPredefinedID', async () => {
const attributes = { attrOne: 'one', attrSecret: 'secret', attrThree: 'three' };
const options = { namespace: 'some-namespace' };
const mockedResponse = {
saved_objects: [
{
id: 'some-id',
type: 'known-type-predefined-id',
attributes,
references: [],
},
{
id: 'some-id',
type: 'unknown-type',
attributes,
references: [],
},
],
};
mockBaseClient.bulkCreate.mockResolvedValue(mockedResponse);
const bulkCreateParams = [
{ id: 'some-id', type: 'known-type-predefined-id', attributes },
{ type: 'unknown-type', attributes },
];
await expect(wrapper.bulkCreate(bulkCreateParams, options)).resolves.toEqual({
saved_objects: [
{ ...mockedResponse.saved_objects[0], attributes: { attrOne: 'one', attrThree: 'three' } },
mockedResponse.saved_objects[1],
],
});
expect(mockBaseClient.bulkCreate).toHaveBeenCalled();
});
it('allows a specified ID when overwriting an existing object', async () => {
const attributes = {
attrOne: 'one',

24
x-pack/plugins/encrypted_saved_objects/server/saved_objects/encrypted_saved_objects_client_wrapper.ts
View file

@ -68,16 +68,14 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon
}
// Saved objects with encrypted attributes should have IDs that are hard to guess especially
// since IDs are part of the AAD used during encryption, that's why we control them within this
// wrapper and don't allow consumers to specify their own IDs directly.
// 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 = options.overwrite && options.version;
if (options.id && !canSpecifyID) {
// since IDs are part of the AAD used during encryption. Types can opt-out of this restriction,
// when necessary, but it's much safer for this wrapper to generate them.
if (
options.id &&
!this.options.service.canSpecifyID(type, options.version, options.overwrite)
) {
throw new Error(
'Predefined IDs are not allowed for saved objects with encrypted attributes.'
`Predefined IDs are not allowed for encrypted saved objects of type "${type}".`
);
}
@ -118,10 +116,12 @@ export class EncryptedSavedObjectsClientWrapper implements SavedObjectsClientCon
// Saved objects with encrypted attributes should have IDs that are hard to guess especially
// since IDs are part of the AAD used during encryption, that's why we control them within this
// wrapper and don't allow consumers to specify their own IDs directly unless overwriting the original document.
const canSpecifyID = options?.overwrite && object.version;
if (object.id && !canSpecifyID) {
if (
object.id &&
!this.options.service.canSpecifyID(object.type, object.version, options?.overwrite)
) {
throw new Error(
'Predefined IDs are not allowed for saved objects with encrypted attributes.'
`Predefined IDs are not allowed for encrypted saved objects of type "${object.type}".`
);
}