Allow additive csp configuration

src/core/server/csp/config.test.ts

@@ -9,11 +9,56 @@
 import { config } from './config';
 
 describe('config.validate()', () => {
-  test(`does not allow "disableEmbedding" to be set to true`, () => {
+  it(`does not allow "disableEmbedding" to be set to true`, () => {
     // This is intentionally not editable in the raw CSP config.
     // Users should set `server.securityResponseHeaders.disableEmbedding` to control this config property.
     expect(() => config.schema.validate({ disableEmbedding: true })).toThrowError(
       '[disableEmbedding]: expected value to equal [false]'
     );
   });
+
+  it('throws if both `rules` and `script_src` are specified', () => {
+    expect(() =>
+      config.schema.validate({
+        rules: [
+          `script-src 'unsafe-eval' 'self'`,
+          `worker-src 'unsafe-eval' 'self'`,
+          `style-src 'unsafe-eval' 'self'`,
+        ],
+        script_src: [`'self'`],
+      })
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"\\"csp.rules\\" cannot be used when specifying per-directive additions such as \\"script_src\\", \\"worker_src\\" or \\"style_src\\""`
+    );
+  });
+
+  it('throws if both `rules` and `worker_src` are specified', () => {
+    expect(() =>
+      config.schema.validate({
+        rules: [
+          `script-src 'unsafe-eval' 'self'`,
+          `worker-src 'unsafe-eval' 'self'`,
+          `style-src 'unsafe-eval' 'self'`,
+        ],
+        worker_src: [`'self'`],
+      })
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"\\"csp.rules\\" cannot be used when specifying per-directive additions such as \\"script_src\\", \\"worker_src\\" or \\"style_src\\""`
+    );
+  });
+
+  it('throws if both `rules` and `style_src` are specified', () => {
+    expect(() =>
+      config.schema.validate({
+        rules: [
+          `script-src 'unsafe-eval' 'self'`,
+          `worker-src 'unsafe-eval' 'self'`,
+          `style-src 'unsafe-eval' 'self'`,
+        ],
+        style_src: [`'self'`],
+      })
+    ).toThrowErrorMatchingInlineSnapshot(
+      `"\\"csp.rules\\" cannot be used when specifying per-directive additions such as \\"script_src\\", \\"worker_src\\" or \\"style_src\\""`
+    );
+  });
 });

src/core/server/csp/config.ts

@@ -7,28 +7,56 @@
  */
 
 import { TypeOf, schema } from '@kbn/config-schema';
+import { ServiceConfigDescriptor } from '../internal_types';
+
+const configSchema = schema.object(
+  {
+    rules: schema.maybe(schema.arrayOf(schema.string())),
+    script_src: schema.arrayOf(schema.string(), { defaultValue: [] }),
+    worker_src: schema.arrayOf(schema.string(), { defaultValue: [] }),
+    style_src: schema.arrayOf(schema.string(), { defaultValue: [] }),
+    strict: schema.boolean({ defaultValue: true }),
+    warnLegacyBrowsers: schema.boolean({ defaultValue: true }),
+    disableEmbedding: schema.oneOf([schema.literal<boolean>(false)], { defaultValue: false }),
+  },
+  {
+    validate: (cspConfig) => {
+      if (
+        cspConfig.rules &&
+        (cspConfig.script_src.length || cspConfig.worker_src.length || cspConfig.style_src.length)
+      ) {
+        return `"csp.rules" cannot be used when specifying per-directive additions such as "script_src", "worker_src" or "style_src"`;
+      }
+    },
+  }
+);
 
 /**
  * @internal
  */
-export type CspConfigType = TypeOf<typeof config.schema>;
+export type CspConfigType = TypeOf<typeof configSchema>;
 
-export const config = {
+export const config: ServiceConfigDescriptor<CspConfigType> = {
   // TODO: Move this to server.csp using config deprecations
   // ? https://github.com/elastic/kibana/pull/52251
   path: 'csp',
-  schema: schema.object({
-    rules: schema.arrayOf(schema.string(), {
-      defaultValue: [
-        `script-src 'unsafe-eval' 'self'`,
-        `worker-src blob: 'self'`,
-        `style-src 'unsafe-inline' 'self'`,
-      ],
-    }),
-    strict: schema.boolean({ defaultValue: true }),
-    warnLegacyBrowsers: schema.boolean({ defaultValue: true }),
-    disableEmbedding: schema.oneOf([schema.literal<boolean>(false)], { defaultValue: false }),
-  }),
+  schema: configSchema,
+  deprecations: () => [
+    (rawConfig, fromPath, addDeprecation) => {
+      const cspConfig = rawConfig[fromPath];
+      if (cspConfig?.rules) {
+        addDeprecation({
+          message:
+            'csp.rules is deprecated in favor of per-directive definitions. ' +
+            'Please use `csp.script_src`, `csp.worker_src` and `csp.style_src` instead',
+          correctiveActions: {
+            manualSteps: [
+              `Remove "csp.rules" from the Kibana config file"`,
+              `Add per-directive definitions to the config file, using "csp.script_src", "csp.worker_src" and "csp.style_src"`,
+            ],
+          },
+        });
+      }
+    },
+  ],
 };
-
-export const FRAME_ANCESTORS_RULE = `frame-ancestors 'self'`; // only used by CspConfig when embedding is disabled

src/core/server/csp/csp_config.test.ts

@@ -7,7 +7,7 @@
  */
 
 import { CspConfig } from './csp_config';
-import { FRAME_ANCESTORS_RULE } from './config';
+import { config as cspConfig, CspConfigType } from './config';
 
 // CSP rules aren't strictly additive, so any change can potentially expand or
 // restrict the policy in a way we consider a breaking change. For that reason,
@@ -23,6 +23,12 @@ import { FRAME_ANCESTORS_RULE } from './config';
 // the nature of a change in defaults during a PR review.
 
 describe('CspConfig', () => {
+  let defaultConfig: CspConfigType;
+
+  beforeEach(() => {
+    defaultConfig = cspConfig.schema.validate({});
+  });
+
   test('DEFAULT', () => {
     expect(CspConfig.DEFAULT).toMatchInlineSnapshot(`
       CspConfig {
@@ -40,26 +46,26 @@ describe('CspConfig', () => {
   });
 
   test('defaults from config', () => {
-    expect(new CspConfig()).toEqual(CspConfig.DEFAULT);
+    expect(new CspConfig(defaultConfig)).toEqual(CspConfig.DEFAULT);
   });
 
   describe('partial config', () => {
     test('allows "rules" to be set and changes header', () => {
-      const rules = ['foo', 'bar'];
-      const config = new CspConfig({ rules });
+      const rules = [`foo 'self'`, `bar 'self'`];
+      const config = new CspConfig({ ...defaultConfig, rules });
       expect(config.rules).toEqual(rules);
-      expect(config.header).toMatchInlineSnapshot(`"foo; bar"`);
+      expect(config.header).toMatchInlineSnapshot(`"foo 'self'; bar 'self'"`);
     });
 
     test('allows "strict" to be set', () => {
-      const config = new CspConfig({ strict: false });
+      const config = new CspConfig({ ...defaultConfig, strict: false });
       expect(config.strict).toEqual(false);
       expect(config.strict).not.toEqual(CspConfig.DEFAULT.strict);
     });
 
     test('allows "warnLegacyBrowsers" to be set', () => {
       const warnLegacyBrowsers = false;
-      const config = new CspConfig({ warnLegacyBrowsers });
+      const config = new CspConfig({ ...defaultConfig, warnLegacyBrowsers });
       expect(config.warnLegacyBrowsers).toEqual(warnLegacyBrowsers);
       expect(config.warnLegacyBrowsers).not.toEqual(CspConfig.DEFAULT.warnLegacyBrowsers);
     });
@@ -68,22 +74,22 @@ describe('CspConfig', () => {
       const disableEmbedding = true;
 
       test('and changes rules/header if custom rules are not defined', () => {
-        const config = new CspConfig({ disableEmbedding });
+        const config = new CspConfig({ ...defaultConfig, disableEmbedding });
         expect(config.disableEmbedding).toEqual(disableEmbedding);
         expect(config.disableEmbedding).not.toEqual(CspConfig.DEFAULT.disableEmbedding);
-        expect(config.rules).toEqual(expect.arrayContaining([FRAME_ANCESTORS_RULE]));
+        expect(config.rules).toEqual(expect.arrayContaining([`frame-ancestors 'self'`]));
         expect(config.header).toMatchInlineSnapshot(
           `"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self'"`
         );
       });
 
       test('and does not change rules/header if custom rules are defined', () => {
-        const rules = ['foo', 'bar'];
-        const config = new CspConfig({ disableEmbedding, rules });
+        const rules = [`foo 'self'`, `bar 'self'`];
+        const config = new CspConfig({ ...defaultConfig, disableEmbedding, rules });
         expect(config.disableEmbedding).toEqual(disableEmbedding);
         expect(config.disableEmbedding).not.toEqual(CspConfig.DEFAULT.disableEmbedding);
         expect(config.rules).toEqual(rules);
-        expect(config.header).toMatchInlineSnapshot(`"foo; bar"`);
+        expect(config.header).toMatchInlineSnapshot(`"foo 'self'; bar 'self'"`);
       });
     });
   });

src/core/server/csp/csp_config.ts

@@ -6,7 +6,8 @@
  * Side Public License, v 1.
  */
 
-import { config, FRAME_ANCESTORS_RULE } from './config';
+import { config, CspConfigType } from './config';
+import { CspDirectives } from './csp_directives';
 
 const DEFAULT_CONFIG = Object.freeze(config.schema.validate({}));
 
@@ -50,8 +51,9 @@ export interface ICspConfig {
  * @public
  */
 export class CspConfig implements ICspConfig {
-  static readonly DEFAULT = new CspConfig();
+  static readonly DEFAULT = new CspConfig(DEFAULT_CONFIG);
 
+  readonly #directives: CspDirectives;
   public readonly rules: string[];
   public readonly strict: boolean;
   public readonly warnLegacyBrowsers: boolean;
@@ -62,16 +64,17 @@ export class CspConfig implements ICspConfig {
    * Returns the default CSP configuration when passed with no config
    * @internal
    */
-  constructor(rawCspConfig: Partial<Omit<ICspConfig, 'header'>> = {}) {
-    const source = { ...DEFAULT_CONFIG, ...rawCspConfig };
-
-    this.rules = [...source.rules];
-    this.strict = source.strict;
-    this.warnLegacyBrowsers = source.warnLegacyBrowsers;
-    this.disableEmbedding = source.disableEmbedding;
-    if (!rawCspConfig.rules?.length && source.disableEmbedding) {
-      this.rules.push(FRAME_ANCESTORS_RULE);
+  constructor(rawCspConfig: CspConfigType) {
+    this.#directives = CspDirectives.fromConfig(rawCspConfig);
+    if (!rawCspConfig.rules?.length && rawCspConfig.disableEmbedding) {
+      this.#directives.addDirectiveValue('frame-ancestors', `'self'`);
     }
-    this.header = this.rules.join('; ');
+
+    this.rules = this.#directives.getRules();
+    this.header = this.#directives.getCspHeader();
+
+    this.strict = rawCspConfig.strict;
+    this.warnLegacyBrowsers = rawCspConfig.warnLegacyBrowsers;
+    this.disableEmbedding = rawCspConfig.disableEmbedding;
   }
 }

src/core/server/csp/csp_directives.test.ts

@@ -0,0 +1,134 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { CspDirectives } from './csp_directives';
+import { config as cspConfig } from './config';
+
+describe('CspDirectives', () => {
+  describe('#addDirectiveValue', () => {
+    it('properly updates the rules', () => {
+      const directives = new CspDirectives();
+      directives.addDirectiveValue('style-src', 'foo');
+
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "style-src foo",
+        ]
+      `);
+
+      directives.addDirectiveValue('style-src', 'bar');
+
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "style-src foo bar",
+        ]
+      `);
+    });
+
+    it('properly updates the header', () => {
+      const directives = new CspDirectives();
+      directives.addDirectiveValue('style-src', 'foo');
+
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(`"style-src foo"`);
+
+      directives.addDirectiveValue('style-src', 'bar');
+
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(`"style-src foo bar"`);
+    });
+
+    it('handles distinct directives', () => {
+      const directives = new CspDirectives();
+      directives.addDirectiveValue('style-src', 'foo');
+      directives.addDirectiveValue('style-src', 'bar');
+      directives.addDirectiveValue('worker-src', 'self');
+
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(
+        `"style-src foo bar; worker-src self"`
+      );
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "style-src foo bar",
+          "worker-src self",
+        ]
+      `);
+    });
+
+    it('removes duplicates', () => {
+      const directives = new CspDirectives();
+      directives.addDirectiveValue('style-src', 'foo');
+      directives.addDirectiveValue('style-src', 'foo');
+      directives.addDirectiveValue('style-src', 'bar');
+
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(`"style-src foo bar"`);
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "style-src foo bar",
+        ]
+      `);
+    });
+  });
+
+  describe('#fromConfig', () => {
+    it('returns the correct rules for the default config', () => {
+      const config = cspConfig.schema.validate({});
+      const directives = CspDirectives.fromConfig(config);
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "script-src 'unsafe-eval' 'self'",
+          "worker-src blob: 'self'",
+          "style-src 'unsafe-inline' 'self'",
+        ]
+      `);
+    });
+
+    it('returns the correct header for the default config', () => {
+      const config = cspConfig.schema.validate({});
+      const directives = CspDirectives.fromConfig(config);
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(
+        `"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'"`
+      );
+    });
+
+    it('handles config with rules', () => {
+      const config = cspConfig.schema.validate({
+        rules: [`script-src 'self' http://foo.com`, `worker-src 'self'`],
+      });
+      const directives = CspDirectives.fromConfig(config);
+
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "script-src 'self' http://foo.com",
+          "worker-src 'self'",
+        ]
+      `);
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(
+        `"script-src 'self' http://foo.com; worker-src 'self'"`
+      );
+    });
+
+    it('adds default value for config with directives', () => {
+      const config = cspConfig.schema.validate({
+        script_src: [`baz`],
+        worker_src: [`foo`],
+        style_src: [`bar`, `dolly`],
+      });
+      const directives = CspDirectives.fromConfig(config);
+
+      expect(directives.getRules()).toMatchInlineSnapshot(`
+        Array [
+          "script-src 'unsafe-eval' 'self' baz",
+          "worker-src blob: 'self' foo",
+          "style-src 'unsafe-inline' 'self' bar dolly",
+        ]
+      `);
+      expect(directives.getCspHeader()).toMatchInlineSnapshot(
+        `"script-src 'unsafe-eval' 'self' baz; worker-src blob: 'self' foo; style-src 'unsafe-inline' 'self' bar dolly"`
+      );
+    });
+  });
+});