Skip to content

Commit

Permalink
cleaned up some crypto code
Browse files Browse the repository at this point in the history
  • Loading branch information
gmmorris committed Jun 24, 2020
1 parent 7e3b377 commit ca13aee
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 51 deletions.
16 changes: 8 additions & 8 deletions x-pack/plugins/encrypted_saved_objects/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const savedObjectWithDecryptedContent = await esoClient.getDecryptedAsInternalU
one would pass to `SavedObjectsClient.get`. These argument allows to specify `namespace` property that, for example, is
required if Saved Object was created within a non-default space.

### defining migrations
### Defining migrations
EncryptedSavedObjects rely on standard SavedObject migrations, but due to the additional complexity introduced by the need to decrypt and reencrypt the migrated document, there are some caveats to how we support this.
The good news is, most of this complexity is abstracted away by the plugin and all you need to do is leverage our api.

Expand All @@ -118,7 +118,7 @@ For example:

```typescript
const migration790 = encryptedSavedObjects.createMigration<RawAlert, RawAlert>(
function shouldbeMigrated(doc): doc is SavedObjectUnsanitizedDoc<RawAlert> {
function shouldBeMigrated(doc): doc is SavedObjectUnsanitizedDoc<RawAlert> {
return doc.consumer === 'alerting' || doc.consumer === undefined;
},
(doc: SavedObjectUnsanitizedDoc<RawAlert>): SavedObjectUnsanitizedDoc<RawAlert> => {
Expand All @@ -135,16 +135,16 @@ const migration790 = encryptedSavedObjects.createMigration<RawAlert, RawAlert>(
},
// type hasn't changed as the field we're updating is not an encrypted one
{
type: 'alert',
attributesToEncrypt: new Set(['apiKey']),
attributesToExcludeFromAAD: new Set(['mutedInstanceIds', 'updatedBy']),
}
type: 'alert',
attributesToEncrypt: new Set(['apiKey']),
attributesToExcludeFromAAD: new Set(['mutedInstanceIds', 'updatedBy']),
}
);
```

In the above example you can see thwe following:
1. In `shouldbeMigrated` we limit the migrated alerts to those whose `consumer` field equals `alerting` or is undefined.
2. In the migration function we then migrate the value of `consumer` to the value we want (`alerts` or `unknown`, dependsing on the current value). In this function we can assume that only documents with a `consumer` of `alerting` or `undefined` will be passed in, but it's still safest not to, and so we use the current `consumer` as the default when needed.
1. In `shouldBeMigrated` we limit the migrated alerts to those whose `consumer` field equals `alerting` or is undefined.
2. In the migration function we then migrate the value of `consumer` to the value we want (`alerts` or `unknown`, depending on the current value). In this function we can assume that only documents with a `consumer` of `alerting` or `undefined` will be passed in, but it's still safest not to, and so we use the current `consumer` as the default when needed.
3. We provide the type, which remains unchanged across this migration and so we can omit the fourth argument.

As we said above, an EncryptedSavedObject migration is a normal SavedObjects migration, and so we can plug it into the underlying SavedObject just like any other kind of migration:
Expand Down
69 changes: 33 additions & 36 deletions x-pack/plugins/encrypted_saved_objects/server/create_migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {
SavedObjectUnsanitizedDoc,
SavedObjectMigrationFn,
SavedObjectMigrationContext,
} from 'kibana/server';
import { pick } from 'lodash';
import { EncryptedSavedObjectAttributesDefinition } from './crypto/encrypted_saved_object_type_definition';
import { EncryptedSavedObjectTypeRegistration, SavedObjectDescriptor } from './crypto';
import { EncryptedSavedObjectsMigrationService } from './crypto/encrypted_saved_objects_migration_service';
} from 'src/core/server';
import {
EncryptedSavedObjectTypeRegistration,
SavedObjectDescriptor,
EncryptedSavedObjectsMigrationService,
EncryptedSavedObjectAttributesDefinition,
} from './crypto';

type SavedObjectOptionalMigrationFn<InputAttributes, MigratedAttributes> = (
doc: SavedObjectUnsanitizedDoc<InputAttributes> | SavedObjectUnsanitizedDoc<InputAttributes>,
Expand All @@ -25,7 +27,7 @@ type IsMigrationNeededPredicate<InputAttributes, MigratedAttributes> = (
| SavedObjectUnsanitizedDoc<MigratedAttributes>
) => encryptedDoc is SavedObjectUnsanitizedDoc<InputAttributes>;

export type CreateESOMigrationFn = <
export type CreateEncryptedSavedObjectsMigrationFn = <
InputAttributes = unknown,
MigratedAttributes = InputAttributes
>(
Expand All @@ -37,7 +39,7 @@ export type CreateESOMigrationFn = <

export const getCreateMigration = (
migrationService: EncryptedSavedObjectsMigrationService
): CreateESOMigrationFn => (
): CreateEncryptedSavedObjectsMigrationFn => (
isMigrationNeededPredicate,
migration,
inputType,
Expand All @@ -51,37 +53,32 @@ export const getCreateMigration = (
const inputTypeDefinition = new EncryptedSavedObjectAttributesDefinition(inputType);
const migratedTypeDefinition = new EncryptedSavedObjectAttributesDefinition(migratedType);
return (encryptedDoc, context) => {
if (isMigrationNeededPredicate(encryptedDoc)) {
const descriptor = pick<SavedObjectDescriptor, SavedObjectUnsanitizedDoc>(
encryptedDoc,
'id',
'type',
'namespace'
);
if (!isMigrationNeededPredicate(encryptedDoc)) {
return encryptedDoc;
}
const descriptor = {
id: encryptedDoc.id,
type: encryptedDoc.type,
namespace: encryptedDoc.namespace,
};

// decrypt the attributes using the input type definition
// then migrate the document
// then encrypt the attributes using the migration type definition
return mapAttributes(
migration(
mapAttributes(encryptedDoc, (inputAttributes) =>
migrationService.decryptAttributes<any>(
descriptor,
inputTypeDefinition,
inputAttributes
)
),
context
// decrypt the attributes using the input type definition
// then migrate the document
// then encrypt the attributes using the migration type definition
return mapAttributes(
migration(
mapAttributes(encryptedDoc, (inputAttributes) =>
migrationService.decryptAttributes<any>(descriptor, inputTypeDefinition, inputAttributes)
),
(migratedAttributes) =>
migrationService.encryptAttributes<any>(
descriptor,
migratedTypeDefinition,
migratedAttributes
)
);
}
return encryptedDoc;
context
),
(migratedAttributes) =>
migrationService.encryptAttributes<any>(
descriptor,
migratedTypeDefinition,
migratedAttributes
)
);
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import nodeCrypto, { Crypto } from '@elastic/node-crypto';
import { EncryptedSavedObjectsMigrationService } from './encrypted_saved_objects_migration_service';
import { EncryptedSavedObjectAttributesDefinition } from './encrypted_saved_object_type_definition';
import { EncryptionError } from './encryption_error';
import { loggingServiceMock } from 'src/core/server/mocks';
import { loggingSystemMock } from 'src/core/server/mocks';

let service: EncryptedSavedObjectsMigrationService;

Expand All @@ -25,7 +25,7 @@ const mockNodeCrypto: jest.Mocked<Crypto> = {
beforeEach(() => {
service = new EncryptedSavedObjectsMigrationService(
mockNodeCrypto,
loggingServiceMock.create().get()
loggingSystemMock.create().get()
);

// Call actual `@elastic/node-crypto` by default, but allow to override implementation in tests.
Expand All @@ -49,7 +49,7 @@ describe('#encryptAttributes', () => {

service = new EncryptedSavedObjectsMigrationService(
mockNodeCrypto,
loggingServiceMock.create().get()
loggingSystemMock.create().get()
);
});

Expand Down Expand Up @@ -575,7 +575,7 @@ describe('#decryptAttributes', () => {
it('fails if encrypted with another encryption key', () => {
service = new EncryptedSavedObjectsMigrationService(
nodeCrypto({ encryptionKey: 'encryption-key-abc*' }),
loggingServiceMock.create().get()
loggingSystemMock.create().get()
);

const type = new EncryptedSavedObjectAttributesDefinition({
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/encrypted_saved_objects/server/crypto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ export {
SavedObjectDescriptor,
} from './encrypted_saved_objects_service';
export { EncryptionError } from './encryption_error';
export { EncryptedSavedObjectsMigrationService } from './encrypted_saved_objects_migration_service';
export { EncryptedSavedObjectAttributesDefinition } from './encrypted_saved_object_type_definition';
6 changes: 3 additions & 3 deletions x-pack/plugins/encrypted_saved_objects/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
EncryptedSavedObjectsService,
EncryptedSavedObjectTypeRegistration,
EncryptionError,
EncryptedSavedObjectsMigrationService,
} from './crypto';
import { EncryptedSavedObjectsAuditLogger } from './audit';
import { setupSavedObjects, ClientInstanciator } from './saved_objects';
import { EncryptedSavedObjectsMigrationService } from './crypto/encrypted_saved_objects_migration_service';
import { getCreateMigration, CreateESOMigrationFn } from './create_migration';
import { getCreateMigration, CreateEncryptedSavedObjectsMigrationFn } from './create_migration';

export interface PluginsSetup {
security?: SecurityPluginSetup;
Expand All @@ -26,7 +26,7 @@ export interface PluginsSetup {
export interface EncryptedSavedObjectsPluginSetup {
registerType: (typeRegistration: EncryptedSavedObjectTypeRegistration) => void;
usingEphemeralEncryptionKey: boolean;
createMigration: CreateESOMigrationFn;
createMigration: CreateEncryptedSavedObjectsMigrationFn;
}

export interface EncryptedSavedObjectsPluginStart {
Expand Down

0 comments on commit ca13aee

Please sign in to comment.