From 44c9611bd9256f069b7dddac4d95c90adcace628 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Mon, 11 Oct 2021 17:49:21 -0700 Subject: [PATCH] [8.0] Remove support for configuring csp.rules (#114379) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../kibana-plugin-core-server.cspconfig.md | 1 - ...bana-plugin-core-server.cspconfig.rules.md | 11 -- ...core-server.icspconfig.disableembedding.md | 2 +- .../kibana-plugin-core-server.icspconfig.md | 3 +- ...ana-plugin-core-server.icspconfig.rules.md | 13 -- docs/migration/migrate_8_0.asciidoc | 6 + docs/setup/settings.asciidoc | 10 +- .../deprecation/core_deprecations.test.ts | 96 ------------ .../config/deprecation/core_deprecations.ts | 59 +------ src/core/server/csp/config.test.ts | 144 ++--------------- src/core/server/csp/config.ts | 41 ----- src/core/server/csp/csp_config.test.ts | 62 ++------ src/core/server/csp/csp_config.ts | 14 +- src/core/server/csp/csp_directives.test.ts | 147 +----------------- src/core/server/csp/csp_directives.ts | 28 +--- .../http_resources_service.test.ts | 8 +- src/core/server/server.api.md | 3 - .../resources/base/bin/kibana-docker | 1 - .../collectors/csp/csp_collector.test.ts | 8 +- 19 files changed, 54 insertions(+), 603 deletions(-) delete mode 100644 docs/development/core/server/kibana-plugin-core-server.cspconfig.rules.md delete mode 100644 docs/development/core/server/kibana-plugin-core-server.icspconfig.rules.md diff --git a/docs/development/core/server/kibana-plugin-core-server.cspconfig.md b/docs/development/core/server/kibana-plugin-core-server.cspconfig.md index 0337a1f4d330..46f24dfda673 100644 --- a/docs/development/core/server/kibana-plugin-core-server.cspconfig.md +++ b/docs/development/core/server/kibana-plugin-core-server.cspconfig.md @@ -24,7 +24,6 @@ The constructor for this class is marked as internal. Third-party code should no | [DEFAULT](./kibana-plugin-core-server.cspconfig.default.md) | static | CspConfig | | | [disableEmbedding](./kibana-plugin-core-server.cspconfig.disableembedding.md) | | boolean | | | [header](./kibana-plugin-core-server.cspconfig.header.md) | | string | | -| [rules](./kibana-plugin-core-server.cspconfig.rules.md) | | string[] | | | [strict](./kibana-plugin-core-server.cspconfig.strict.md) | | boolean | | | [warnLegacyBrowsers](./kibana-plugin-core-server.cspconfig.warnlegacybrowsers.md) | | boolean | | diff --git a/docs/development/core/server/kibana-plugin-core-server.cspconfig.rules.md b/docs/development/core/server/kibana-plugin-core-server.cspconfig.rules.md deleted file mode 100644 index 2bc73345fe0d..000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.cspconfig.rules.md +++ /dev/null @@ -1,11 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [CspConfig](./kibana-plugin-core-server.cspconfig.md) > [rules](./kibana-plugin-core-server.cspconfig.rules.md) - -## CspConfig.rules property - -Signature: - -```typescript -readonly rules: string[]; -``` diff --git a/docs/development/core/server/kibana-plugin-core-server.icspconfig.disableembedding.md b/docs/development/core/server/kibana-plugin-core-server.icspconfig.disableembedding.md index 2cfd680459fb..42b177c348af 100644 --- a/docs/development/core/server/kibana-plugin-core-server.icspconfig.disableembedding.md +++ b/docs/development/core/server/kibana-plugin-core-server.icspconfig.disableembedding.md @@ -4,7 +4,7 @@ ## ICspConfig.disableEmbedding property -Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled \*and\* no custom rules have been defined, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. +Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. Signature: diff --git a/docs/development/core/server/kibana-plugin-core-server.icspconfig.md b/docs/development/core/server/kibana-plugin-core-server.icspconfig.md index ee49950df076..9da31cdc11e3 100644 --- a/docs/development/core/server/kibana-plugin-core-server.icspconfig.md +++ b/docs/development/core/server/kibana-plugin-core-server.icspconfig.md @@ -16,9 +16,8 @@ export interface ICspConfig | Property | Type | Description | | --- | --- | --- | -| [disableEmbedding](./kibana-plugin-core-server.icspconfig.disableembedding.md) | boolean | Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled \*and\* no custom rules have been defined, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. | +| [disableEmbedding](./kibana-plugin-core-server.icspconfig.disableembedding.md) | boolean | Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. | | [header](./kibana-plugin-core-server.icspconfig.header.md) | string | The CSP rules in a formatted directives string for use in a Content-Security-Policy header. | -| [rules](./kibana-plugin-core-server.icspconfig.rules.md) | string[] | The CSP rules used for Kibana. | | [strict](./kibana-plugin-core-server.icspconfig.strict.md) | boolean | Specify whether browsers that do not support CSP should be able to use Kibana. Use true to block and false to allow. | | [warnLegacyBrowsers](./kibana-plugin-core-server.icspconfig.warnlegacybrowsers.md) | boolean | Specify whether users with legacy browsers should be warned about their lack of Kibana security compliance. | diff --git a/docs/development/core/server/kibana-plugin-core-server.icspconfig.rules.md b/docs/development/core/server/kibana-plugin-core-server.icspconfig.rules.md deleted file mode 100644 index 808eefc30b64..000000000000 --- a/docs/development/core/server/kibana-plugin-core-server.icspconfig.rules.md +++ /dev/null @@ -1,13 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [ICspConfig](./kibana-plugin-core-server.icspconfig.md) > [rules](./kibana-plugin-core-server.icspconfig.rules.md) - -## ICspConfig.rules property - -The CSP rules used for Kibana. - -Signature: - -```typescript -readonly rules: string[]; -``` diff --git a/docs/migration/migrate_8_0.asciidoc b/docs/migration/migrate_8_0.asciidoc index 60a65580501a..dc6754fba1ff 100644 --- a/docs/migration/migrate_8_0.asciidoc +++ b/docs/migration/migrate_8_0.asciidoc @@ -48,6 +48,12 @@ for example, `logstash-*`. *Impact:* To allow Kibana to function for these legacy browsers, set `csp.strict: false`. Since this is about enforcing a security protocol, we *strongly discourage* disabling `csp.strict` unless it is critical that you support Internet Explorer 11. +[float] +==== Configuring content security policy rules is no longer supported +*Details:* Configuring `csp.rules` is removed in favor of per-directive specific configuration. Configuring the default `csp.script_src`, `csp.workers_src` and `csp.style_src` values is not required. + +*Impact:* Configure per-directive sources instead. See https://github.com/elastic/kibana/pull/102059 for more details. + [float] ==== Default logging timezone is now the system's timezone *Details:* In prior releases the timezone used in logs defaulted to UTC. We now use the host machine's timezone by default. diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 9c3d4fc29f13..48bf5fe2cd7b 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -26,13 +26,6 @@ Toggling this causes the server to regenerate assets on the next startup, which may cause a delay before pages start being served. Set to `false` to disable Console. *Default: `true`* -| `csp.rules:` - | deprecated:[7.14.0,"In 8.0 and later, this setting will no longer be supported."] -A https://w3c.github.io/webappsec-csp/[Content Security Policy] template -that disables certain unnecessary and potentially insecure capabilities in -the browser. It is strongly recommended that you keep the default CSP rules -that ship with {kib}. - | `csp.script_src:` | Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src[Content Security Policy `script-src` directive]. @@ -502,8 +495,7 @@ To disable, set to `null`. *Default:* `null` | Controls whether the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy[`Content-Security-Policy`] and https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options[`X-Frame-Options`] headers are configured to disable embedding {kib} in other webpages using iframes. When set to `true`, secure headers are used to disable embedding, which adds the `frame-ancestors: -'self'` directive to the `Content-Security-Policy` response header (if you are using the default CSP rules), and adds the `X-Frame-Options: -SAMEORIGIN` response header. *Default:* `false` +'self'` directive to the `Content-Security-Policy` response header and adds the `X-Frame-Options: SAMEORIGIN` response header. *Default:* `false` | `server.customResponseHeaders:` {ess-icon} | Header names and values to diff --git a/src/core/server/config/deprecation/core_deprecations.test.ts b/src/core/server/config/deprecation/core_deprecations.test.ts index 95e23561a937..e08f2216f5cb 100644 --- a/src/core/server/config/deprecation/core_deprecations.test.ts +++ b/src/core/server/config/deprecation/core_deprecations.test.ts @@ -83,100 +83,4 @@ describe('core deprecations', () => { expect(messages).toHaveLength(0); }); }); - - describe('cspRulesDeprecation', () => { - describe('with nonce source', () => { - it('logs a warning', () => { - const settings = { - csp: { - rules: [`script-src 'self' 'nonce-{nonce}'`], - }, - }; - const { messages } = applyCoreDeprecations(settings); - expect(messages).toMatchInlineSnapshot(` - Array [ - "csp.rules no longer supports the {nonce} syntax. Replacing with 'self' in script-src", - ] - `); - }); - - it('replaces a nonce', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'nonce-{nonce}'`] } }).migrated.csp - .rules - ).toEqual([`script-src 'self'`]); - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'unsafe-eval' 'nonce-{nonce}'`] } }) - .migrated.csp.rules - ).toEqual([`script-src 'unsafe-eval' 'self'`]); - }); - - it('removes a quoted nonce', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'self' 'nonce-{nonce}'`] } }).migrated - .csp.rules - ).toEqual([`script-src 'self'`]); - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'nonce-{nonce}' 'self'`] } }).migrated - .csp.rules - ).toEqual([`script-src 'self'`]); - }); - - it('removes a non-quoted nonce', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'self' nonce-{nonce}`] } }).migrated - .csp.rules - ).toEqual([`script-src 'self'`]); - expect( - applyCoreDeprecations({ csp: { rules: [`script-src nonce-{nonce} 'self'`] } }).migrated - .csp.rules - ).toEqual([`script-src 'self'`]); - }); - - it('removes a strange nonce', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'self' blah-{nonce}-wow`] } }).migrated - .csp.rules - ).toEqual([`script-src 'self'`]); - }); - - it('removes multiple nonces', () => { - expect( - applyCoreDeprecations({ - csp: { - rules: [ - `script-src 'nonce-{nonce}' 'self' blah-{nonce}-wow`, - `style-src 'nonce-{nonce}' 'self'`, - ], - }, - }).migrated.csp.rules - ).toEqual([`script-src 'self'`, `style-src 'self'`]); - }); - }); - - describe('without self source', () => { - it('logs a warning', () => { - const { messages } = applyCoreDeprecations({ - csp: { rules: [`script-src 'unsafe-eval'`] }, - }); - expect(messages).toMatchInlineSnapshot(` - Array [ - "csp.rules must contain the 'self' source. Automatically adding to script-src.", - ] - `); - }); - - it('adds self', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`script-src 'unsafe-eval'`] } }).migrated.csp.rules - ).toEqual([`script-src 'unsafe-eval' 'self'`]); - }); - }); - - it('does not add self to other policies', () => { - expect( - applyCoreDeprecations({ csp: { rules: [`worker-src blob:`] } }).migrated.csp.rules - ).toEqual([`worker-src blob:`]); - }); - }); }); diff --git a/src/core/server/config/deprecation/core_deprecations.ts b/src/core/server/config/deprecation/core_deprecations.ts index 4e5f711fe9f3..5adbb338b42e 100644 --- a/src/core/server/config/deprecation/core_deprecations.ts +++ b/src/core/server/config/deprecation/core_deprecations.ts @@ -45,64 +45,7 @@ const rewriteCorsSettings: ConfigDeprecation = (settings, fromPath, addDeprecati } }; -const cspRulesDeprecation: ConfigDeprecation = (settings, fromPath, addDeprecation) => { - const NONCE_STRING = `{nonce}`; - // Policies that should include the 'self' source - const SELF_POLICIES = Object.freeze(['script-src', 'style-src']); - const SELF_STRING = `'self'`; - - const rules: string[] = settings.csp?.rules; - if (rules) { - const parsed = new Map( - rules.map((ruleStr) => { - const parts = ruleStr.split(/\s+/); - return [parts[0], parts.slice(1)]; - }) - ); - - return { - set: [ - { - path: 'csp.rules', - value: [...parsed].map(([policy, sourceList]) => { - if (sourceList.find((source) => source.includes(NONCE_STRING))) { - addDeprecation({ - message: `csp.rules no longer supports the {nonce} syntax. Replacing with 'self' in ${policy}`, - correctiveActions: { - manualSteps: [`Replace {nonce} syntax with 'self' in ${policy}`], - }, - }); - sourceList = sourceList.filter((source) => !source.includes(NONCE_STRING)); - - // Add 'self' if not present - if (!sourceList.find((source) => source.includes(SELF_STRING))) { - sourceList.push(SELF_STRING); - } - } - - if ( - SELF_POLICIES.includes(policy) && - !sourceList.find((source) => source.includes(SELF_STRING)) - ) { - addDeprecation({ - message: `csp.rules must contain the 'self' source. Automatically adding to ${policy}.`, - correctiveActions: { - manualSteps: [`Add 'self' source to ${policy}.`], - }, - }); - sourceList.push(SELF_STRING); - } - - return `${policy} ${sourceList.join(' ')}`.trim(); - }), - }, - ], - }; - } -}; - -export const coreDeprecationProvider: ConfigDeprecationProvider = ({ rename, unusedFromRoot }) => [ +export const coreDeprecationProvider: ConfigDeprecationProvider = () => [ rewriteCorsSettings, rewriteBasePathDeprecation, - cspRulesDeprecation, ]; diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index 6db93addb7da..346caf488431 100644 --- a/src/core/server/csp/config.test.ts +++ b/src/core/server/csp/config.test.ts @@ -80,21 +80,6 @@ describe('config.validate()', () => { ).not.toThrow(); }); - it(`throws if 'rules' is also 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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -104,6 +89,7 @@ describe('config.validate()', () => { `"[script_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -124,21 +110,6 @@ describe('config.validate()', () => { }); describe(`"worker_src"`, () => { - it(`throws if 'rules' is also 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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -148,6 +119,7 @@ describe('config.validate()', () => { `"[worker_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -168,21 +140,6 @@ describe('config.validate()', () => { }); describe(`"style_src"`, () => { - it(`throws if 'rules' is also 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\\""` - ); - }); - it('throws if using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -192,6 +149,7 @@ describe('config.validate()', () => { `"[style_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -212,21 +170,6 @@ describe('config.validate()', () => { }); describe(`"connect_src"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - connect_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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -236,6 +179,7 @@ describe('config.validate()', () => { `"[connect_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -256,21 +200,6 @@ describe('config.validate()', () => { }); describe(`"default_src"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - default_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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -280,6 +209,7 @@ describe('config.validate()', () => { `"[default_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -300,21 +230,6 @@ describe('config.validate()', () => { }); describe(`"font_src"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - font_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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -324,6 +239,7 @@ describe('config.validate()', () => { `"[font_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -344,21 +260,6 @@ describe('config.validate()', () => { }); describe(`"frame_src"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - frame_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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -368,6 +269,7 @@ describe('config.validate()', () => { `"[frame_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -388,21 +290,6 @@ describe('config.validate()', () => { }); describe(`"img_src"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - img_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 using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -412,6 +299,7 @@ describe('config.validate()', () => { `"[img_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ @@ -432,21 +320,6 @@ describe('config.validate()', () => { }); describe(`"frame_ancestors"`, () => { - it(`throws if 'rules' is also specified`, () => { - expect(() => - config.schema.validate({ - rules: [ - `script-src 'unsafe-eval' 'self'`, - `worker-src 'unsafe-eval' 'self'`, - `style-src 'unsafe-eval' 'self'`, - ], - frame_ancestors: [`'self'`], - }) - ).toThrowErrorMatchingInlineSnapshot( - `"\\"csp.rules\\" cannot be used when specifying per-directive additions such as \\"script_src\\", \\"worker_src\\" or \\"style_src\\""` - ); - }); - it('throws if using an `nonce-*` value', () => { expect(() => config.schema.validate({ @@ -456,6 +329,7 @@ describe('config.validate()', () => { `"[frame_ancestors]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index 3a7cb20985ce..22aa11e4e0c3 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -39,7 +39,6 @@ const getDirectiveValueValidator = ({ allowNone, allowNonce }: DirectiveValidati const configSchema = schema.object( { - rules: schema.maybe(schema.arrayOf(schema.string())), script_src: schema.arrayOf(schema.string(), { defaultValue: [], validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), @@ -89,9 +88,6 @@ const configSchema = schema.object( }, { validate: (cspConfig) => { - if (cspConfig.rules && hasDirectiveSpecified(cspConfig)) { - return `"csp.rules" cannot be used when specifying per-directive additions such as "script_src", "worker_src" or "style_src"`; - } const hasUnsafeInlineScriptSrc = cspConfig.script_src.includes(`unsafe-inline`) || cspConfig.script_src.includes(`'unsafe-inline'`); @@ -106,22 +102,6 @@ const configSchema = schema.object( } ); -const hasDirectiveSpecified = (rawConfig: CspConfigType): boolean => { - return Boolean( - rawConfig.script_src.length || - rawConfig.worker_src.length || - rawConfig.style_src.length || - rawConfig.connect_src.length || - rawConfig.default_src.length || - rawConfig.font_src.length || - rawConfig.frame_src.length || - rawConfig.img_src.length || - rawConfig.frame_ancestors.length || - rawConfig.report_uri.length || - rawConfig.report_to.length - ); -}; - /** * @internal */ @@ -132,25 +112,4 @@ export const config: ServiceConfigDescriptor = { // ? https://github.com/elastic/kibana/pull/52251 path: 'csp', schema: configSchema, - deprecations: () => [ - (rawConfig, fromPath, addDeprecation) => { - const cspConfig = rawConfig[fromPath]; - if (cspConfig?.rules) { - addDeprecation({ - message: - '`csp.rules` is deprecated in favor of directive specific configuration. Please use `csp.connect_src`, ' + - '`csp.default_src`, `csp.font_src`, `csp.frame_ancestors`, `csp.frame_src`, `csp.img_src`, ' + - '`csp.report_uri`, `csp.report_to`, `csp.script_src`, `csp.style_src`, and `csp.worker_src` instead.', - correctiveActions: { - manualSteps: [ - `Remove "csp.rules" from the Kibana config file."`, - `Add directive specific configurations to the config file using "csp.connect_src", "csp.default_src", "csp.font_src", ` + - `"csp.frame_ancestors", "csp.frame_src", "csp.img_src", "csp.report_uri", "csp.report_to", "csp.script_src", ` + - `"csp.style_src", and "csp.worker_src".`, - ], - }, - }); - } - }, - ], }; diff --git a/src/core/server/csp/csp_config.test.ts b/src/core/server/csp/csp_config.test.ts index a1bac7d4ae73..1ec78fae7532 100644 --- a/src/core/server/csp/csp_config.test.ts +++ b/src/core/server/csp/csp_config.test.ts @@ -34,11 +34,6 @@ describe('CspConfig', () => { CspConfig { "disableEmbedding": false, "header": "script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", - "rules": Array [ - "script-src 'unsafe-eval' 'self'", - "worker-src blob: 'self'", - "style-src 'unsafe-inline' 'self'", - ], "strict": true, "warnLegacyBrowsers": true, } @@ -50,13 +45,6 @@ describe('CspConfig', () => { }); describe('partial config', () => { - test('allows "rules" to be set and changes header', () => { - const rules = [`foo 'self'`, `bar 'self'`]; - const config = new CspConfig({ ...defaultConfig, rules }); - expect(config.rules).toEqual(rules); - expect(config.header).toMatchInlineSnapshot(`"foo 'self'; bar 'self'"`); - }); - test('allows "strict" to be set', () => { const config = new CspConfig({ ...defaultConfig, strict: false }); expect(config.strict).toEqual(false); @@ -70,67 +58,57 @@ describe('CspConfig', () => { expect(config.warnLegacyBrowsers).not.toEqual(CspConfig.DEFAULT.warnLegacyBrowsers); }); - test('allows "worker_src" to be set and changes header', () => { + test('allows "worker_src" to be set and changes header from defaults', () => { 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`); + expect(config.header).toEqual( + `script-src 'unsafe-eval' 'self'; worker-src blob: 'self' foo bar; style-src 'unsafe-inline' 'self'` + ); }); 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`); + + expect(config.header).toEqual( + `script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self' 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`); + + expect(config.header).toEqual( + `script-src 'unsafe-eval' 'self' foo bar; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + ); }); 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` + `script-src 'unsafe-eval' 'self' script foo; worker-src blob: 'self' worker bar; style-src 'unsafe-inline' 'self' style dolly` ); }); - test('applies defaults when `rules` is undefined', () => { + test('appends config directives to defaults', () => { 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` ); @@ -139,25 +117,15 @@ describe('CspConfig', () => { describe('allows "disableEmbedding" to be set', () => { const disableEmbedding = true; - test('and changes rules/header if custom rules are not defined', () => { + test('and changes rules and header', () => { 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 '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 '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 'self'; bar 'self'"`); - }); - test('and overrides `frame-ancestors` if set', () => { const config = new CspConfig({ ...defaultConfig, diff --git a/src/core/server/csp/csp_config.ts b/src/core/server/csp/csp_config.ts index 13778088d9df..f0f968ecd0ce 100644 --- a/src/core/server/csp/csp_config.ts +++ b/src/core/server/csp/csp_config.ts @@ -16,11 +16,6 @@ const DEFAULT_CONFIG = Object.freeze(config.schema.validate({})); * @public */ export interface ICspConfig { - /** - * The CSP rules used for Kibana. - */ - readonly rules: string[]; - /** * Specify whether browsers that do not support CSP should be * able to use Kibana. Use `true` to block and `false` to allow. @@ -34,8 +29,7 @@ export interface ICspConfig { readonly warnLegacyBrowsers: boolean; /** - * Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled *and* no custom rules have been - * defined, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. + * Whether or not embedding (using iframes) should be allowed by the CSP. If embedding is disabled, a restrictive 'frame-ancestors' rule will be added to the default CSP rules. */ readonly disableEmbedding: boolean; @@ -54,7 +48,6 @@ export class CspConfig implements ICspConfig { static readonly DEFAULT = new CspConfig(DEFAULT_CONFIG); readonly #directives: CspDirectives; - public readonly rules: string[]; public readonly strict: boolean; public readonly warnLegacyBrowsers: boolean; public readonly disableEmbedding: boolean; @@ -66,14 +59,11 @@ export class CspConfig implements ICspConfig { */ constructor(rawCspConfig: CspConfigType) { this.#directives = CspDirectives.fromConfig(rawCspConfig); - if (!rawCspConfig.rules?.length && rawCspConfig.disableEmbedding) { + if (rawCspConfig.disableEmbedding) { this.#directives.clearDirectiveValues('frame-ancestors'); this.#directives.addDirectiveValue('frame-ancestors', `'self'`); } - - this.rules = this.#directives.getRules(); this.header = this.#directives.getCspHeader(); - this.strict = rawCspConfig.strict; this.warnLegacyBrowsers = rawCspConfig.warnLegacyBrowsers; this.disableEmbedding = rawCspConfig.disableEmbedding; diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts index 1077b6ea9f3c..f4a9e256e2f9 100644 --- a/src/core/server/csp/csp_directives.test.ts +++ b/src/core/server/csp/csp_directives.test.ts @@ -11,33 +11,12 @@ 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"`); }); @@ -50,12 +29,6 @@ describe('CspDirectives', () => { expect(directives.getCspHeader()).toMatchInlineSnapshot( `"style-src foo bar; worker-src dolly"` ); - expect(directives.getRules()).toMatchInlineSnapshot(` - Array [ - "style-src foo bar", - "worker-src dolly", - ] - `); }); it('removes duplicates', () => { @@ -65,11 +38,6 @@ describe('CspDirectives', () => { directives.addDirectiveValue('style-src', 'bar'); expect(directives.getCspHeader()).toMatchInlineSnapshot(`"style-src foo bar"`); - expect(directives.getRules()).toMatchInlineSnapshot(` - Array [ - "style-src foo bar", - ] - `); }); it('automatically adds single quotes for keywords', () => { @@ -106,18 +74,6 @@ describe('CspDirectives', () => { }); 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); @@ -126,75 +82,6 @@ describe('CspDirectives', () => { ); }); - 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 single quotes for keyword for 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('handles multiple whitespaces when parsing 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('supports unregistered directives', () => { - const config = cspConfig.schema.validate({ - rules: [`script-src 'self' http://foo.com`, `img-src 'self'`, 'foo bar'], - }); - const directives = CspDirectives.fromConfig(config); - - expect(directives.getRules()).toMatchInlineSnapshot(` - Array [ - "script-src 'self' http://foo.com", - "img-src 'self'", - "foo bar", - ] - `); - expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"script-src 'self' http://foo.com; img-src 'self'; foo bar"` - ); - }); - it('adds default value for config with directives', () => { const config = cspConfig.schema.validate({ script_src: [`baz`], @@ -203,13 +90,6 @@ describe('CspDirectives', () => { }); 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"` ); @@ -227,22 +107,9 @@ describe('CspDirectives', () => { report_to: [`report-to`], }); const directives = CspDirectives.fromConfig(config); - - expect(directives.getRules()).toMatchInlineSnapshot(` - Array [ - "script-src 'unsafe-eval' 'self'", - "worker-src blob: 'self'", - "style-src 'unsafe-inline' 'self'", - "connect-src 'self' connect-src", - "default-src 'self' default-src", - "font-src 'self' font-src", - "frame-src 'self' frame-src", - "img-src 'self' img-src", - "frame-ancestors 'self' frame-ancestors", - "report-uri report-uri", - "report-to report-to", - ] - `); + expect(directives.getCspHeader()).toMatchInlineSnapshot( + `"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; connect-src 'self' connect-src; default-src 'self' default-src; font-src 'self' font-src; frame-src 'self' frame-src; img-src 'self' img-src; frame-ancestors 'self' frame-ancestors; report-uri report-uri; report-to report-to"` + ); }); it('adds single quotes for keywords in added directives', () => { @@ -250,14 +117,6 @@ describe('CspDirectives', () => { script_src: [`unsafe-hashes`], }); const directives = CspDirectives.fromConfig(config); - - expect(directives.getRules()).toMatchInlineSnapshot(` - Array [ - "script-src 'unsafe-eval' 'self' 'unsafe-hashes'", - "worker-src blob: 'self'", - "style-src 'unsafe-inline' 'self'", - ] - `); expect(directives.getCspHeader()).toMatchInlineSnapshot( `"script-src 'unsafe-eval' 'self' 'unsafe-hashes'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'"` ); diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index 9e3b60f7f1e4..d656210a054f 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -22,7 +22,7 @@ export type CspDirectiveName = | 'report-to'; /** - * The default rules that are always applied + * The default directives rules that are always applied */ export const defaultRules: Partial> = { 'script-src': [`'unsafe-eval'`, `'self'`], @@ -58,21 +58,18 @@ export class CspDirectives { } getCspHeader() { - return this.getRules().join('; '); - } - - getRules() { - return [...this.directives.entries()].map(([name, values]) => { - return [name, ...values].join(' '); - }); + return [...this.directives.entries()] + .map(([name, values]) => { + return [name, ...values].join(' '); + }) + .join('; '); } static fromConfig(config: CspConfigType): CspDirectives { const cspDirectives = new CspDirectives(); - // adding `csp.rules` or `default` rules - const initialRules = config.rules ? parseRules(config.rules) : { ...defaultRules }; - Object.entries(initialRules).forEach(([key, values]) => { + // combining `default` directive configurations + Object.entries(defaultRules).forEach(([key, values]) => { values?.forEach((value) => { cspDirectives.addDirectiveValue(key as CspDirectiveName, value); }); @@ -91,15 +88,6 @@ export class CspDirectives { } } -const parseRules = (rules: string[]): Partial> => { - const directives: Partial> = {}; - rules.forEach((rule) => { - const [name, ...values] = rule.replace(/\s+/g, ' ').trim().split(' '); - directives[name as CspDirectiveName] = values; - }); - return directives; -}; - const parseConfigDirectives = (cspConfig: CspConfigType): Map => { const map = new Map(); diff --git a/src/core/server/http_resources/integration_tests/http_resources_service.test.ts b/src/core/server/http_resources/integration_tests/http_resources_service.test.ts index 6f4f3c9c6e98..3b254df92903 100644 --- a/src/core/server/http_resources/integration_tests/http_resources_service.test.ts +++ b/src/core/server/http_resources/integration_tests/http_resources_service.test.ts @@ -12,12 +12,10 @@ import * as kbnTestServer from '../../../test_helpers/kbn_server'; describe('http resources service', () => { describe('register', () => { let root: ReturnType; - const defaultCspRules = "script-src 'self'"; + const defaultCspRules = + "script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'"; beforeEach(async () => { root = kbnTestServer.createRoot({ - csp: { - rules: [defaultCspRules], - }, plugins: { initialize: false }, elasticsearch: { skipStartupConnectionCheck: true }, }); @@ -44,7 +42,7 @@ describe('http resources service', () => { expect(response.text.length).toBeGreaterThan(0); }); - it('attaches CSP header', async () => { + it('applies default CSP header', async () => { const { http, httpResources } = await root.setup(); const router = http.createRouter(''); diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index a1fd69f5e1c7..fb16e889a19c 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -754,8 +754,6 @@ export class CspConfig implements ICspConfig { // (undocumented) readonly header: string; // (undocumented) - readonly rules: string[]; - // (undocumented) readonly strict: boolean; // (undocumented) readonly warnLegacyBrowsers: boolean; @@ -1135,7 +1133,6 @@ export type IContextProvider { expect((await collector.fetch(mockedFetchContext)).warnLegacyBrowsers).toEqual(false); }); - test('fetches whether the csp rules have been changed or not', async () => { + test("fetches whether the csp directives's rules have been changed or not", async () => { const collector = new Collector(logger, createCspCollector(httpMock)); expect((await collector.fetch(mockedFetchContext)).rulesChangedFromDefault).toEqual(false); - updateCsp({ rules: ['not', 'default'] }); + updateCsp({ disableEmbedding: true }); expect((await collector.fetch(mockedFetchContext)).rulesChangedFromDefault).toEqual(true); }); test('does not include raw csp rules under any property names', async () => { const collector = new Collector(logger, createCspCollector(httpMock)); - // It's important that we do not send the value of csp.rules here as it + // It's important that we do not send the raw values of csp cirectives here as they // can be customized with values that can be identifiable to given // installs, such as URLs // - // We use a snapshot here to ensure csp.rules isn't finding its way into the + // We use a snapshot here to ensure raw values aren't finding their way into the // payload under some new and unexpected variable name (e.g. cspRules). expect(await collector.fetch(mockedFetchContext)).toMatchInlineSnapshot(` Object {