Disable EncryptedSavedObjects plugin in production if encryption key …

…is not specified.
elastic · Feb 5, 2021 · 23777af · 23777af
1 parent 1f0da4f
commit 23777af
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 85 deletions.
diff --git a/x-pack/plugins/encrypted_saved_objects/server/config.test.ts b/x-pack/plugins/encrypted_saved_objects/server/config.test.ts
@@ -5,10 +5,7 @@
  * 2.0.
  */
 
-jest.mock('crypto', () => ({ randomBytes: jest.fn() }));
-
-import { loggingSystemMock } from 'src/core/server/mocks';
-import { createConfig, ConfigSchema } from './config';
+import { ConfigSchema } from './config';
 
 describe('config schema', () => {
   it('generates proper defaults', () => {
@@ -32,9 +29,20 @@ describe('config schema', () => {
       }
     `);
 
-    expect(ConfigSchema.validate({}, { dist: true })).toMatchInlineSnapshot(`
+    expect(ConfigSchema.validate({ encryptionKey: 'z'.repeat(32) }, { dist: true }))
+      .toMatchInlineSnapshot(`
       Object {
         "enabled": true,
+        "encryptionKey": "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz",
+        "keyRotation": Object {
+          "decryptionOnlyKeys": Array [],
+        },
+      }
+    `);
+
+    expect(ConfigSchema.validate({}, { dist: true })).toMatchInlineSnapshot(`
+      Object {
+        "enabled": false,
         "keyRotation": Object {
           "decryptionOnlyKeys": Array [],
         },
@@ -46,15 +54,15 @@ describe('config schema', () => {
     expect(
       ConfigSchema.validate(
         {
-          encryptionKey: 'a'.repeat(32),
+          encryptionKey: 'z'.repeat(32),
           keyRotation: { decryptionOnlyKeys: ['b'.repeat(32), 'c'.repeat(32)] },
         },
         { dist: true }
       )
     ).toMatchInlineSnapshot(`
       Object {
         "enabled": true,
-        "encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+        "encryptionKey": "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz",
         "keyRotation": Object {
           "decryptionOnlyKeys": Array [
             "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
@@ -79,6 +87,14 @@ describe('config schema', () => {
     );
   });
 
+  it('should throw error if `enabled` is set to `true`, but xpack.encryptedSavedObjects.encryptionKey is not specified', () => {
+    expect(() =>
+      ConfigSchema.validate({ enabled: true }, { dist: true })
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"\`enabled\` cannot be set to \`true\` until \`encryptionKey\` is specified."`
+    );
+  });
+
   it('should throw error if any of the xpack.encryptedSavedObjects.keyRotation.decryptionOnlyKeys is less than 32 characters', () => {
     expect(() =>
       ConfigSchema.validate({
@@ -121,43 +137,3 @@ describe('config schema', () => {
     );
   });
 });
-
-describe('createConfig()', () => {
-  it('should log a warning, set xpack.encryptedSavedObjects.encryptionKey and usingEphemeralEncryptionKey=true when encryptionKey is not set', () => {
-    const mockRandomBytes = jest.requireMock('crypto').randomBytes;
-    mockRandomBytes.mockReturnValue('ab'.repeat(16));
-
-    const logger = loggingSystemMock.create().get();
-    const config = createConfig(ConfigSchema.validate({}, { dist: true }), logger);
-    expect(config).toEqual({
-      enabled: true,
-      encryptionKey: 'ab'.repeat(16),
-      keyRotation: { decryptionOnlyKeys: [] },
-      usingEphemeralEncryptionKey: true,
-    });
-
-    expect(loggingSystemMock.collect(logger).warn).toMatchInlineSnapshot(`
-      Array [
-        Array [
-          "Generating a random key for xpack.encryptedSavedObjects.encryptionKey. To decrypt encrypted saved objects attributes after restart, please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.",
-        ],
-      ]
-    `);
-  });
-
-  it('should not log a warning and set usingEphemeralEncryptionKey=false when encryptionKey is set', async () => {
-    const logger = loggingSystemMock.create().get();
-    const config = createConfig(
-      ConfigSchema.validate({ encryptionKey: 'supersecret'.repeat(3) }, { dist: true }),
-      logger
-    );
-    expect(config).toEqual({
-      enabled: true,
-      encryptionKey: 'supersecret'.repeat(3),
-      keyRotation: { decryptionOnlyKeys: [] },
-      usingEphemeralEncryptionKey: false,
-    });
-
-    expect(loggingSystemMock.collect(logger).warn).toEqual([]);
-  });
-});
diff --git a/x-pack/plugins/encrypted_saved_objects/server/config.ts b/x-pack/plugins/encrypted_saved_objects/server/config.ts
@@ -5,15 +5,20 @@
  * 2.0.
  */
 
-import crypto from 'crypto';
 import { schema, TypeOf } from '@kbn/config-schema';
-import { Logger } from 'src/core/server';
 
-export type ConfigType = ReturnType<typeof createConfig>;
+export type ConfigType = Omit<TypeOf<typeof ConfigSchema>, 'encryptionKey'> & {
+  encryptionKey: string;
+};
 
 export const ConfigSchema = schema.object(
   {
-    enabled: schema.boolean({ defaultValue: true }),
+    enabled: schema.conditional(
+      schema.siblingRef('encryptionKey'),
+      schema.string({ minLength: 32 }),
+      schema.boolean({ defaultValue: true }),
+      schema.boolean({ defaultValue: false })
+    ),
     encryptionKey: schema.conditional(
       schema.contextRef('dist'),
       true,
@@ -30,26 +35,10 @@ export const ConfigSchema = schema.object(
       if (value.encryptionKey && decryptionOnlyKeys.includes(value.encryptionKey)) {
         return '`keyRotation.decryptionOnlyKeys` cannot contain primary encryption key specified in `encryptionKey`.';
       }
+
+      if (value.enabled && !value.encryptionKey) {
+        return '`enabled` cannot be set to `true` until `encryptionKey` is specified.';
+      }
     },
   }
 );
-
-export function createConfig(config: TypeOf<typeof ConfigSchema>, logger: Logger) {
-  let encryptionKey = config.encryptionKey;
-  const usingEphemeralEncryptionKey = encryptionKey === undefined;
-  if (encryptionKey === undefined) {
-    logger.warn(
-      'Generating a random key for xpack.encryptedSavedObjects.encryptionKey. ' +
-        'To decrypt encrypted saved objects attributes after restart, ' +
-        'please set xpack.encryptedSavedObjects.encryptionKey in the kibana.yml or use the bin/kibana-encryption-keys command.'
-    );
-
-    encryptionKey = crypto.randomBytes(16).toString('hex');
-  }
-
-  return {
-    ...config,
-    encryptionKey,
-    usingEphemeralEncryptionKey,
-  };
-}
diff --git a/x-pack/plugins/encrypted_saved_objects/server/plugin.test.ts b/x-pack/plugins/encrypted_saved_objects/server/plugin.test.ts
@@ -15,14 +15,15 @@ describe('EncryptedSavedObjects Plugin', () => {
   describe('setup()', () => {
     it('exposes proper contract', async () => {
       const plugin = new Plugin(
-        coreMock.createPluginInitializerContext(ConfigSchema.validate({}, { dist: true }))
+        coreMock.createPluginInitializerContext(
+          ConfigSchema.validate({ encryptionKey: 'z'.repeat(32) }, { dist: true })
+        )
       );
       await expect(plugin.setup(coreMock.createSetup(), { security: securityMock.createSetup() }))
         .resolves.toMatchInlineSnapshot(`
               Object {
                 "createMigration": [Function],
                 "registerType": [Function],
-                "usingEphemeralEncryptionKey": true,
               }
             `);
     });
@@ -31,7 +32,9 @@ describe('EncryptedSavedObjects Plugin', () => {
   describe('start()', () => {
     it('exposes proper contract', async () => {
       const plugin = new Plugin(
-        coreMock.createPluginInitializerContext(ConfigSchema.validate({}, { dist: true }))
+        coreMock.createPluginInitializerContext(
+          ConfigSchema.validate({ encryptionKey: 'z'.repeat(32) }, { dist: true })
+        )
       );
       await plugin.setup(coreMock.createSetup(), { security: securityMock.createSetup() });
 

diff --git a/x-pack/plugins/encrypted_saved_objects/server/plugin.ts b/x-pack/plugins/encrypted_saved_objects/server/plugin.ts
@@ -5,12 +5,11 @@
  * 2.0.
  */
 
-import { first, map } from 'rxjs/operators';
+import { first } from 'rxjs/operators';
 import nodeCrypto from '@elastic/node-crypto';
 import { Logger, PluginInitializerContext, CoreSetup } from 'src/core/server';
-import { TypeOf } from '@kbn/config-schema';
 import { SecurityPluginSetup } from '../../security/server';
-import { createConfig, ConfigSchema } from './config';
+import { ConfigType } from './config';
 import {
   EncryptedSavedObjectsService,
   EncryptedSavedObjectTypeRegistration,
@@ -28,7 +27,6 @@ export interface PluginsSetup {
 
 export interface EncryptedSavedObjectsPluginSetup {
   registerType: (typeRegistration: EncryptedSavedObjectTypeRegistration) => void;
-  usingEphemeralEncryptionKey: boolean;
   createMigration: CreateEncryptedSavedObjectsMigrationFn;
 }
 
@@ -53,10 +51,7 @@ export class Plugin {
     deps: PluginsSetup
   ): Promise<EncryptedSavedObjectsPluginSetup> {
     const config = await this.initializerContext.config
-      .create<TypeOf<typeof ConfigSchema>>()
-      .pipe(
-        map((rawConfig) => createConfig(rawConfig, this.initializerContext.logger.get('config')))
-      )
+      .create<ConfigType>()
       .pipe(first())
       .toPromise();
     const auditLogger = new EncryptedSavedObjectsAuditLogger(
@@ -101,7 +96,6 @@ export class Plugin {
     return {
       registerType: (typeRegistration: EncryptedSavedObjectTypeRegistration) =>
         service.registerType(typeRegistration),
-      usingEphemeralEncryptionKey: config.usingEphemeralEncryptionKey,
       createMigration: getCreateMigration(
         service,
         (typeRegistration: EncryptedSavedObjectTypeRegistration) => {

diff --git a/x-pack/plugins/encrypted_saved_objects/server/routes/index.mock.ts b/x-pack/plugins/encrypted_saved_objects/server/routes/index.mock.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { ConfigSchema, createConfig } from '../config';
+import { ConfigSchema, ConfigType } from '../config';
 
 import { httpServiceMock, loggingSystemMock } from '../../../../../src/core/server/mocks';
 import { encryptionKeyRotationServiceMock } from '../crypto/index.mock';
@@ -14,7 +14,7 @@ export const routeDefinitionParamsMock = {
   create: (config: Record<string, unknown> = {}) => ({
     router: httpServiceMock.createRouter(),
     logger: loggingSystemMock.create().get(),
-    config: createConfig(ConfigSchema.validate(config), loggingSystemMock.create().get()),
+    config: ConfigSchema.validate(config) as ConfigType,
     encryptionKeyRotationService: encryptionKeyRotationServiceMock.create(),
   }),
 };