Allow additive csp configuration

docs/development/core/server/kibana-plugin-core-server.cspconfig.__private_.md

@@ -0,0 +1,11 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [CspConfig](./kibana-plugin-core-server.cspconfig.md) &gt; ["\#private"](./kibana-plugin-core-server.cspconfig.__private_.md)
+
+## CspConfig."\#private" property
+
+<b>Signature:</b>
+
+```typescript
+#private;
+```

docs/development/core/server/kibana-plugin-core-server.cspconfig.md

@@ -20,6 +20,7 @@ The constructor for this class is marked as internal. Third-party code should no
 
 |  Property | Modifiers | Type | Description |
 |  --- | --- | --- | --- |
+|  ["\#private"](./kibana-plugin-core-server.cspconfig.__private_.md) |  | <code></code> |  |
 |  [DEFAULT](./kibana-plugin-core-server.cspconfig.default.md) | <code>static</code> | <code>CspConfig</code> |  |
 |  [disableEmbedding](./kibana-plugin-core-server.cspconfig.disableembedding.md) |  | <code>boolean</code> |  |
 |  [header](./kibana-plugin-core-server.cspconfig.header.md) |  | <code>string</code> |  |

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,50 +46,116 @@ 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);
     });
 
+    test('allows "worker_src" to be set and changes header', () => {
+      const config = new CspConfig({
+        ...defaultConfig,
+        rules: [],
+        worker_src: ['foo', 'bar'],
+      });
+      expect(config.rules).toEqual([`worker-src foo bar`]);
+      expect(config.header).toEqual(`worker-src foo bar`);
+    });
+
+    test('allows "style_src" to be set and changes header', () => {
+      const config = new CspConfig({
+        ...defaultConfig,
+        rules: [],
+        style_src: ['foo', 'bar'],
+      });
+      expect(config.rules).toEqual([`style-src foo bar`]);
+      expect(config.header).toEqual(`style-src foo bar`);
+    });
+
+    test('allows "script_src" to be set and changes header', () => {
+      const config = new CspConfig({
+        ...defaultConfig,
+        rules: [],
+        script_src: ['foo', 'bar'],
+      });
+      expect(config.rules).toEqual([`script-src foo bar`]);
+      expect(config.header).toEqual(`script-src foo bar`);
+    });
+
+    test('allows all directives to be set and changes header', () => {
+      const config = new CspConfig({
+        ...defaultConfig,
+        rules: [],
+        script_src: ['script', 'foo'],
+        worker_src: ['worker', 'bar'],
+        style_src: ['style', 'dolly'],
+      });
+      expect(config.rules).toEqual([
+        `script-src script foo`,
+        `worker-src worker bar`,
+        `style-src style dolly`,
+      ]);
+      expect(config.header).toEqual(
+        `script-src script foo; worker-src worker bar; style-src style dolly`
+      );
+    });
+
+    test('applies defaults when `rules` is undefined', () => {
+      const config = new CspConfig({
+        ...defaultConfig,
+        rules: undefined,
+        script_src: ['script'],
+        worker_src: ['worker'],
+        style_src: ['style'],
+      });
+      expect(config.rules).toEqual([
+        `script-src 'unsafe-eval' 'self' script`,
+        `worker-src blob: 'self' worker`,
+        `style-src 'unsafe-inline' 'self' style`,
+      ]);
+      expect(config.header).toEqual(
+        `script-src 'unsafe-eval' 'self' script; worker-src blob: 'self' worker; style-src 'unsafe-inline' 'self' style`
+      );
+    });
+
     describe('allows "disableEmbedding" to be set', () => {
       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;
   }
 }