From 80ecdedc4a96323eb5863fb6577b8a1d8a44dfed Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 14 Jun 2021 15:02:16 +0200 Subject: [PATCH 01/23] add additive csp configuration --- src/core/server/csp/config.test.ts | 47 ++++++++++++++++++- src/core/server/csp/config.ts | 60 +++++++++++++++++------- src/core/server/csp/csp_config.test.ts | 30 +++++++----- src/core/server/csp/csp_config.ts | 27 ++++++----- src/core/server/csp/csp_directives.ts | 65 ++++++++++++++++++++++++++ 5 files changed, 188 insertions(+), 41 deletions(-) create mode 100644 src/core/server/csp/csp_directives.ts diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index 8036ebeeaad313..bba41ab6649531 100644 --- a/src/core/server/csp/config.test.ts +++ b/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\\""` + ); + }); }); diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index a61fa1b03a45c8..97a612b60ecced 100644 --- a/src/core/server/csp/config.ts +++ b/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(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; +export type CspConfigType = TypeOf; -export const config = { +export const config: ServiceConfigDescriptor = { // 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(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 diff --git a/src/core/server/csp/csp_config.test.ts b/src/core/server/csp/csp_config.test.ts index 1e023c6f08ea80..c05e58b470c2ab 100644 --- a/src/core/server/csp/csp_config.test.ts +++ b/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'"`); }); }); }); diff --git a/src/core/server/csp/csp_config.ts b/src/core/server/csp/csp_config.ts index 649c81576ef522..05e3d9edc7433d 100644 --- a/src/core/server/csp/csp_config.ts +++ b/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> = {}) { - 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; } } diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts new file mode 100644 index 00000000000000..5e094a4250011f --- /dev/null +++ b/src/core/server/csp/csp_directives.ts @@ -0,0 +1,65 @@ +/* + * 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 { CspConfigType } from './config'; + +export type CspDirectiveName = 'script-src' | 'worker-src' | 'style-src' | 'frame-ancestors'; + +export const defaultRules: Partial> = { + 'script-src': [`'unsafe-eval'`, `'self'`], + 'worker-src': [`blob:`, `'self'`], + 'style-src': [`'unsafe-inline'`, `'self'`], +}; + +export class CspDirectives { + private readonly directives = new Map(); + + addDirectiveValue(directiveName: CspDirectiveName, directiveValue: string) { + const directives = this.directives.get(directiveName) ?? []; + this.directives.set(directiveName, [...directives, directiveValue]); + } + + getCspHeader() { + return this.getRules().join('; '); + } + + getRules() { + return [...this.directives.entries()].map(([name, values]) => { + return [name, ...values].join(' '); + }); + } + + static fromConfig(config: CspConfigType): CspDirectives { + const cspDirectives = new CspDirectives(); + const initialRules = config.rules ? parseRules(config.rules) : { ...defaultRules }; + Object.entries(initialRules).forEach(([key, values]) => { + values?.forEach((value) => { + cspDirectives.addDirectiveValue(key as CspDirectiveName, value); + }); + }); + config.script_src.forEach((scriptSrc) => { + cspDirectives.addDirectiveValue('script-src', scriptSrc); + }); + config.worker_src.forEach((workerSrc) => { + cspDirectives.addDirectiveValue('worker-src', workerSrc); + }); + config.style_src.forEach((styleSrc) => { + cspDirectives.addDirectiveValue('style-src', styleSrc); + }); + return cspDirectives; + } +} + +const parseRules = (rules: string[]): Partial> => { + const directives: Partial> = {}; + rules.forEach((rule) => { + const [name, ...values] = rule.trim().split(' '); + directives[name as CspDirectiveName] = values; + }); + return directives; +}; From 73890df2f22606aa6cceca4c84f2e1452a2aa1ea Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 14 Jun 2021 16:44:22 +0200 Subject: [PATCH 02/23] add unit tests for new class --- src/core/server/csp/csp_directives.test.ts | 134 +++++++++++++++++++++ src/core/server/csp/csp_directives.ts | 8 +- 2 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 src/core/server/csp/csp_directives.test.ts diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts new file mode 100644 index 00000000000000..7f6f3a2caa2a68 --- /dev/null +++ b/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"` + ); + }); + }); +}); diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index 5e094a4250011f..1c4b97d2249c34 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -17,11 +17,13 @@ export const defaultRules: Partial> = { }; export class CspDirectives { - private readonly directives = new Map(); + private readonly directives = new Map>(); addDirectiveValue(directiveName: CspDirectiveName, directiveValue: string) { - const directives = this.directives.get(directiveName) ?? []; - this.directives.set(directiveName, [...directives, directiveValue]); + if (!this.directives.has(directiveName)) { + this.directives.set(directiveName, new Set()); + } + this.directives.get(directiveName)!.add(directiveValue); } getCspHeader() { From 303d60a64a183d86db2864bd81a456a93904e6a7 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 14 Jun 2021 16:46:35 +0200 Subject: [PATCH 03/23] fix types --- src/core/server/http/http_config.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/server/http/http_config.test.ts b/src/core/server/http/http_config.test.ts index 56095336d970b9..06a47456322332 100644 --- a/src/core/server/http/http_config.test.ts +++ b/src/core/server/http/http_config.test.ts @@ -8,7 +8,7 @@ import uuid from 'uuid'; import { config, HttpConfig } from './http_config'; -import { CspConfig } from '../csp'; +import { config as cspConfig } from '../csp'; import { ExternalUrlConfig } from '../external_url'; const validHostnames = ['www.example.com', '8.8.8.8', '::1', 'localhost', '0.0.0.0']; @@ -459,7 +459,8 @@ describe('HttpConfig', () => { }, }, }); - const httpConfig = new HttpConfig(rawConfig, CspConfig.DEFAULT, ExternalUrlConfig.DEFAULT); + const rawCspConfig = cspConfig.schema.validate({}); + const httpConfig = new HttpConfig(rawConfig, rawCspConfig, ExternalUrlConfig.DEFAULT); expect(httpConfig.customResponseHeaders).toEqual({ string: 'string', From b3d60b2f1516066df96e8315df3e4d5a88b25e88 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 14 Jun 2021 16:49:08 +0200 Subject: [PATCH 04/23] adapt test utils --- src/core/server/http/test_utils.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/server/http/test_utils.ts b/src/core/server/http/test_utils.ts index b3180b43d0026a..8a8c1f771bc171 100644 --- a/src/core/server/http/test_utils.ts +++ b/src/core/server/http/test_utils.ts @@ -56,7 +56,11 @@ configService.atPath.mockImplementation((path) => { } as any); } if (path === 'csp') { - return new BehaviorSubject({} as any); + return new BehaviorSubject({ + script_src: [], + worker_src: [], + style_src: [], + } as any); } throw new Error(`Unexpected config path: ${path}`); }); From ee9f805a49fe5dd2637b2cd1bf0ff695387fe687 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 14 Jun 2021 20:06:16 +0200 Subject: [PATCH 05/23] fix tests --- src/core/server/http/cookie_session_storage.test.ts | 10 +++++++++- .../http/integration_tests/lifecycle_handlers.test.ts | 10 +++++++++- src/core/server/http/test_utils.ts | 6 +++++- .../server/collectors/csp/csp_collector.test.ts | 8 +++++++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/core/server/http/cookie_session_storage.test.ts b/src/core/server/http/cookie_session_storage.test.ts index c8021638664234..3f67ee66995095 100644 --- a/src/core/server/http/cookie_session_storage.test.ts +++ b/src/core/server/http/cookie_session_storage.test.ts @@ -16,6 +16,7 @@ import { CoreContext } from '../core_context'; import { HttpService } from './http_service'; import { KibanaRequest } from './router'; import { Env } from '../config'; +import type { CspConfigType } from '../csp'; import { contextServiceMock } from '../context/context_service.mock'; import { loggingSystemMock } from '../logging/logging_system.mock'; @@ -69,7 +70,14 @@ configService.atPath.mockImplementation((path) => { } as any); } if (path === 'csp') { - return new BehaviorSubject({} as any); + return new BehaviorSubject({ + script_src: [], + worker_src: [], + style_src: [], + strict: false, + disableEmbedding: false, + warnLegacyBrowsers: true, + } as CspConfigType); } throw new Error(`Unexpected config path: ${path}`); }); diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts index cbd300fdc9c09b..7d17afe95f9d4a 100644 --- a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts +++ b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts @@ -11,6 +11,7 @@ import moment from 'moment'; import { BehaviorSubject } from 'rxjs'; import { ByteSizeValue } from '@kbn/config-schema'; +import type { CspConfigType } from '../../csp'; import { createHttpServer } from '../test_utils'; import { HttpService } from '../http_service'; import { HttpServerSetup } from '../http_server'; @@ -79,7 +80,14 @@ describe('core lifecycle handlers', () => { } as any); } if (path === 'csp') { - return new BehaviorSubject({} as any); + return new BehaviorSubject({ + script_src: [], + worker_src: [], + style_src: [], + strict: false, + disableEmbedding: false, + warnLegacyBrowsers: true, + } as CspConfigType); } throw new Error(`Unexpected config path: ${path}`); }); diff --git a/src/core/server/http/test_utils.ts b/src/core/server/http/test_utils.ts index 8a8c1f771bc171..d39b2d62a4689e 100644 --- a/src/core/server/http/test_utils.ts +++ b/src/core/server/http/test_utils.ts @@ -15,6 +15,7 @@ import { HttpService } from './http_service'; import { CoreContext } from '../core_context'; import { getEnvOptions, configServiceMock } from '../config/mocks'; import { loggingSystemMock } from '../logging/logging_system.mock'; +import type { CspConfigType } from '../csp'; const coreId = Symbol('core'); const env = Env.createDefault(REPO_ROOT, getEnvOptions()); @@ -60,7 +61,10 @@ configService.atPath.mockImplementation((path) => { script_src: [], worker_src: [], style_src: [], - } as any); + strict: false, + disableEmbedding: false, + warnLegacyBrowsers: true, + } as CspConfigType); } throw new Error(`Unexpected config path: ${path}`); }); diff --git a/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts b/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts index fffbe013d98e87..6244fa561a4e3f 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts @@ -22,7 +22,13 @@ describe('csp collector', () => { const mockedFetchContext = createCollectorFetchContextMock(); function updateCsp(config: Partial) { - httpMock.csp = new CspConfig(config); + httpMock.csp = new CspConfig({ + ...CspConfig.DEFAULT, + style_src: [], + worker_src: [], + script_src: [], + ...config, + }); } beforeEach(() => { From dca375fc29558609d37bd0255cac73cef3ed99b1 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 15 Jun 2021 08:16:59 +0200 Subject: [PATCH 06/23] more unit tests on config --- src/core/server/csp/csp_config.test.ts | 66 ++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/core/server/csp/csp_config.test.ts b/src/core/server/csp/csp_config.test.ts index c05e58b470c2ab..217d97979ac404 100644 --- a/src/core/server/csp/csp_config.test.ts +++ b/src/core/server/csp/csp_config.test.ts @@ -70,6 +70,72 @@ describe('CspConfig', () => { 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; From 4b4511106d6ab6de6d80dfbe3ef9f763dea11194 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 15 Jun 2021 09:06:19 +0200 Subject: [PATCH 07/23] generated doc --- .../kibana-plugin-core-server.cspconfig.__private_.md | 11 +++++++++++ .../server/kibana-plugin-core-server.cspconfig.md | 1 + src/core/server/server.api.md | 6 +++++- 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.cspconfig.__private_.md diff --git a/docs/development/core/server/kibana-plugin-core-server.cspconfig.__private_.md b/docs/development/core/server/kibana-plugin-core-server.cspconfig.__private_.md new file mode 100644 index 00000000000000..217066481d33c1 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.cspconfig.__private_.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [CspConfig](./kibana-plugin-core-server.cspconfig.md) > ["\#private"](./kibana-plugin-core-server.cspconfig.__private_.md) + +## CspConfig."\#private" property + +Signature: + +```typescript +#private; +``` 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 9f4f3211ea2b1c..0337a1f4d33014 100644 --- a/docs/development/core/server/kibana-plugin-core-server.cspconfig.md +++ b/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) | | | | | [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 | | diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index ce13174ee19cc2..8e6ecc330f698d 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -777,8 +777,12 @@ export interface CountResponse { // @public export class CspConfig implements ICspConfig { + // (undocumented) + #private; + // Warning: (ae-forgotten-export) The symbol "CspConfigType" needs to be exported by the entry point index.d.ts + // // @internal - constructor(rawCspConfig?: Partial>); + constructor(rawCspConfig: CspConfigType); // (undocumented) static readonly DEFAULT: CspConfig; // (undocumented) From ca5616db89452106ac362dcf28fc6c0c99c839e5 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 15 Jun 2021 14:57:49 +0200 Subject: [PATCH 08/23] review comments --- src/core/server/csp/config.ts | 4 +-- src/core/server/csp/csp_directives.test.ts | 35 ++++++++++++++++++++++ src/core/server/csp/csp_directives.ts | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index 97a612b60ecced..15dea5a89dc00b 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -47,12 +47,12 @@ export const config: ServiceConfigDescriptor = { if (cspConfig?.rules) { addDeprecation({ message: - 'csp.rules is deprecated in favor of per-directive definitions. ' + + 'csp.rules is deprecated in favor of directive specific configuration. ' + '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"`, + `Add directive specific configurations to the config file, using "csp.script_src", "csp.worker_src" and "csp.style_src"`, ], }, }); diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts index 7f6f3a2caa2a68..1b5a40e0662492 100644 --- a/src/core/server/csp/csp_directives.test.ts +++ b/src/core/server/csp/csp_directives.test.ts @@ -111,6 +111,41 @@ describe('CspDirectives', () => { ); }); + 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`], diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index 1c4b97d2249c34..f4676f1c4282c7 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -60,7 +60,7 @@ export class CspDirectives { const parseRules = (rules: string[]): Partial> => { const directives: Partial> = {}; rules.forEach((rule) => { - const [name, ...values] = rule.trim().split(' '); + const [name, ...values] = rule.replace(/\s+/g, ' ').trim().split(' '); directives[name as CspDirectiveName] = values; }); return directives; From 0d3bd2cc7ddf7bd04da63a5dfcd181b81f6128d0 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 15 Jun 2021 15:12:06 +0200 Subject: [PATCH 09/23] update ascii doc --- docs/setup/settings.asciidoc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index ddb906f390a2d9..85e7c9b4a4a21d 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -36,11 +36,21 @@ Set to `false` to disable Console. *Default: `true`* <>. | `csp.rules:` - | A https://w3c.github.io/webappsec-csp/[content-security-policy] template + | 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 values for the `script-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. + +| `csp.worker_src:` +| Add values for the `worker-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. + +| `csp.style_src:` +| Add values for the `style-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. + |[[csp-strict]] `csp.strict:` | Blocks {kib} access to any browser that does not enforce even rudimentary CSP rules. In practice, this disables From e1eb1b50f27e65672a9e0892f2abe5235a4245be Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 16 Jun 2021 08:16:57 +0200 Subject: [PATCH 10/23] update ascii doc links --- docs/setup/settings.asciidoc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 85e7c9b4a4a21d..fc0791ae660913 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -37,19 +37,19 @@ 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 +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 values for the `script-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. +| 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]. | `csp.worker_src:` -| Add values for the `worker-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/worker-src[Content Security Policy `worker-src` directive]. | `csp.style_src:` -| Add values for the `style-src` https://w3c.github.io/webappsec-csp/[content-security-policy] directive. +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src[Content Security Policy `style-src` directive]. |[[csp-strict]] `csp.strict:` | Blocks {kib} access to any browser that From 42c81ec6019ed55b7be0ac6bc6118ddd7bb926a1 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 16 Jun 2021 09:01:47 +0200 Subject: [PATCH 11/23] automatically add single quotes for keywords --- src/core/server/csp/csp_directives.test.ts | 73 +++++++++++++++++++++- src/core/server/csp/csp_directives.ts | 20 +++++- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts index 1b5a40e0662492..a94bfd1fc90372 100644 --- a/src/core/server/csp/csp_directives.test.ts +++ b/src/core/server/csp/csp_directives.test.ts @@ -45,15 +45,15 @@ describe('CspDirectives', () => { const directives = new CspDirectives(); directives.addDirectiveValue('style-src', 'foo'); directives.addDirectiveValue('style-src', 'bar'); - directives.addDirectiveValue('worker-src', 'self'); + directives.addDirectiveValue('worker-src', 'dolly'); expect(directives.getCspHeader()).toMatchInlineSnapshot( - `"style-src foo bar; worker-src self"` + `"style-src foo bar; worker-src dolly"` ); expect(directives.getRules()).toMatchInlineSnapshot(` Array [ "style-src foo bar", - "worker-src self", + "worker-src dolly", ] `); }); @@ -71,6 +71,38 @@ describe('CspDirectives', () => { ] `); }); + + it('automatically adds single quotes for keywords', () => { + const directives = new CspDirectives(); + directives.addDirectiveValue('style-src', 'none'); + directives.addDirectiveValue('style-src', 'self'); + directives.addDirectiveValue('style-src', 'strict-dynamic'); + directives.addDirectiveValue('style-src', 'report-sample'); + directives.addDirectiveValue('style-src', 'unsafe-inline'); + directives.addDirectiveValue('style-src', 'unsafe-eval'); + directives.addDirectiveValue('style-src', 'unsafe-hashes'); + directives.addDirectiveValue('style-src', 'unsafe-allow-redirects'); + + expect(directives.getCspHeader()).toMatchInlineSnapshot( + `"style-src 'none' 'self' 'strict-dynamic' 'report-sample' 'unsafe-inline' 'unsafe-eval' 'unsafe-hashes' 'unsafe-allow-redirects'"` + ); + }); + + it('does not add single quotes for keywords when already present', () => { + const directives = new CspDirectives(); + directives.addDirectiveValue('style-src', `'none'`); + directives.addDirectiveValue('style-src', `'self'`); + directives.addDirectiveValue('style-src', `'strict-dynamic'`); + directives.addDirectiveValue('style-src', `'report-sample'`); + directives.addDirectiveValue('style-src', `'unsafe-inline'`); + directives.addDirectiveValue('style-src', `'unsafe-eval'`); + directives.addDirectiveValue('style-src', `'unsafe-hashes'`); + directives.addDirectiveValue('style-src', `'unsafe-allow-redirects'`); + + expect(directives.getCspHeader()).toMatchInlineSnapshot( + `"style-src 'none' 'self' 'strict-dynamic' 'report-sample' 'unsafe-inline' 'unsafe-eval' 'unsafe-hashes' 'unsafe-allow-redirects'"` + ); + }); }); describe('#fromConfig', () => { @@ -111,6 +143,23 @@ describe('CspDirectives', () => { ); }); + 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' `], @@ -165,5 +214,23 @@ describe('CspDirectives', () => { `"script-src 'unsafe-eval' 'self' baz; worker-src blob: 'self' foo; style-src 'unsafe-inline' 'self' bar dolly"` ); }); + + it('adds single quotes for keywords in added directives', () => { + const config = cspConfig.schema.validate({ + 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 f4676f1c4282c7..69b40c0bff08cc 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -23,7 +23,7 @@ export class CspDirectives { if (!this.directives.has(directiveName)) { this.directives.set(directiveName, new Set()); } - this.directives.get(directiveName)!.add(directiveValue); + this.directives.get(directiveName)!.add(normalizeDirectiveValue(directiveValue)); } getCspHeader() { @@ -65,3 +65,21 @@ const parseRules = (rules: string[]): Partial }); return directives; }; + +const keywordTokens = [ + 'none', + 'self', + 'strict-dynamic', + 'report-sample', + 'unsafe-inline', + 'unsafe-eval', + 'unsafe-hashes', + 'unsafe-allow-redirects', +]; + +function normalizeDirectiveValue(value: string) { + if (keywordTokens.includes(value)) { + return `'${value}'`; + } + return value; +} From 592a1198a9fd9abf2e022612d9e11f2282f7f49a Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 22 Jun 2021 21:44:15 +0200 Subject: [PATCH 12/23] add missing csp directives --- src/core/server/csp/config.ts | 42 +++++++++-- src/core/server/csp/csp_directives.test.ts | 30 ++++++++ src/core/server/csp/csp_directives.ts | 86 +++++++++++++++++++--- 3 files changed, 142 insertions(+), 16 deletions(-) diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index 15dea5a89dc00b..533579d438fe59 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -9,28 +9,56 @@ import { TypeOf, schema } from '@kbn/config-schema'; import { ServiceConfigDescriptor } from '../internal_types'; +const validateDirectiveValues = (values: string[]) => {}; + +const cspDirectiveSchema = schema.arrayOf(schema.string(), { + defaultValue: [], + validate: validateDirectiveValues, +}); + 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: [] }), + script_src: cspDirectiveSchema, + worker_src: cspDirectiveSchema, + style_src: cspDirectiveSchema, + connect_src: cspDirectiveSchema, + default_src: cspDirectiveSchema, + font_src: cspDirectiveSchema, + frame_src: cspDirectiveSchema, + img_src: cspDirectiveSchema, + frame_ancestors: cspDirectiveSchema, + report_uri: cspDirectiveSchema, + report_to: cspDirectiveSchema, strict: schema.boolean({ defaultValue: true }), warnLegacyBrowsers: schema.boolean({ defaultValue: true }), disableEmbedding: schema.oneOf([schema.literal(false)], { defaultValue: false }), }, { validate: (cspConfig) => { - if ( - cspConfig.rules && - (cspConfig.script_src.length || cspConfig.worker_src.length || cspConfig.style_src.length) - ) { + 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 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 */ diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts index a94bfd1fc90372..36016804b6558c 100644 --- a/src/core/server/csp/csp_directives.test.ts +++ b/src/core/server/csp/csp_directives.test.ts @@ -215,6 +215,36 @@ describe('CspDirectives', () => { ); }); + it('adds additional values for some directives without defaults', () => { + const config = cspConfig.schema.validate({ + connect_src: [`connect-src`], + default_src: [`default-src`], + font_src: [`font-src`], + frame_src: [`frame-src`], + img_src: [`img-src`], + frame_ancestors: [`frame-ancestors`], + report_uri: [`report-uri`], + 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 frame-src", + "img-src 'self' img-src", + "frame-ancestors frame-ancestors", + "report-uri report-uri", + "report-to report-to", + ] + `); + }); + it('adds single quotes for keywords in added directives', () => { const config = cspConfig.schema.validate({ script_src: [`unsafe-hashes`], diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index 69b40c0bff08cc..e60e49237ea14d 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -8,14 +8,39 @@ import { CspConfigType } from './config'; -export type CspDirectiveName = 'script-src' | 'worker-src' | 'style-src' | 'frame-ancestors'; +export type CspDirectiveName = + | 'script-src' + | 'worker-src' + | 'style-src' + | 'frame-ancestors' + | 'connect-src' + | 'default-src' + | 'font-src' + | 'frame-src' + | 'img-src' + | 'report-uri' + | 'report-to'; +/** + * The default rules that are always applied + */ export const defaultRules: Partial> = { 'script-src': [`'unsafe-eval'`, `'self'`], 'worker-src': [`blob:`, `'self'`], 'style-src': [`'unsafe-inline'`, `'self'`], }; +/** + * Per-directive rules that will be added when the configuration contains at least one value + * Main purpose is to add `self` value to some directives when the configuration specifies other values + */ +export const additionalRules: Partial> = { + 'connect-src': [`'self'`], + 'default-src': [`'self'`], + 'font-src': [`'self'`], + 'img-src': [`'self'`], +}; + export class CspDirectives { private readonly directives = new Map>(); @@ -38,21 +63,24 @@ export class CspDirectives { 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]) => { values?.forEach((value) => { cspDirectives.addDirectiveValue(key as CspDirectiveName, value); }); }); - config.script_src.forEach((scriptSrc) => { - cspDirectives.addDirectiveValue('script-src', scriptSrc); - }); - config.worker_src.forEach((workerSrc) => { - cspDirectives.addDirectiveValue('worker-src', workerSrc); - }); - config.style_src.forEach((styleSrc) => { - cspDirectives.addDirectiveValue('style-src', styleSrc); + + // adding per-directive configuration + const additiveConfig = parseConfigDirectives(config); + [...additiveConfig.entries()].forEach(([directiveName, directiveValues]) => { + const additionalValues = additionalRules[directiveName] ?? []; + [...additionalValues, ...directiveValues].forEach((value) => { + cspDirectives.addDirectiveValue(directiveName, value); + }); }); + return cspDirectives; } } @@ -66,6 +94,46 @@ const parseRules = (rules: string[]): Partial return directives; }; +const parseConfigDirectives = (cspConfig: CspConfigType): Map => { + const map = new Map(); + + if (cspConfig.script_src.length) { + map.set('script-src', cspConfig.script_src); + } + if (cspConfig.worker_src.length) { + map.set('worker-src', cspConfig.worker_src); + } + if (cspConfig.style_src.length) { + map.set('style-src', cspConfig.style_src); + } + if (cspConfig.connect_src.length) { + map.set('connect-src', cspConfig.connect_src); + } + if (cspConfig.default_src.length) { + map.set('default-src', cspConfig.default_src); + } + if (cspConfig.font_src.length) { + map.set('font-src', cspConfig.font_src); + } + if (cspConfig.frame_src.length) { + map.set('frame-src', cspConfig.frame_src); + } + if (cspConfig.img_src.length) { + map.set('img-src', cspConfig.img_src); + } + if (cspConfig.frame_ancestors.length) { + map.set('frame-ancestors', cspConfig.frame_ancestors); + } + if (cspConfig.report_uri.length) { + map.set('report-uri', cspConfig.report_uri); + } + if (cspConfig.report_to.length) { + map.set('report-to', cspConfig.report_to); + } + + return map; +}; + const keywordTokens = [ 'none', 'self', From 383aa95271053f1eac6e749adbce8a582a84274b Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 22 Jun 2021 23:05:35 +0200 Subject: [PATCH 13/23] add more tests --- src/core/server/csp/config.test.ts | 441 ++++++++++++++++++++++++++--- src/core/server/csp/config.ts | 93 ++++-- 2 files changed, 479 insertions(+), 55 deletions(-) diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index bba41ab6649531..fab83b4e8d8cc7 100644 --- a/src/core/server/csp/config.test.ts +++ b/src/core/server/csp/config.test.ts @@ -17,48 +17,411 @@ describe('config.validate()', () => { ); }); - 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\\""` - ); + describe(`"script_src"`, () => { + it(`throws if containing 'unsafe-inline' when 'strict' is true`, () => { + expect(() => + config.schema.validate({ + strict: true, + script_src: [`'self'`, `unsafe-inline`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + ); + + expect(() => + config.schema.validate({ + strict: true, + script_src: [`'self'`, `'unsafe-inline'`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + ); + }); + + it(`does not throw if containing 'unsafe-inline' when 'strict' is false`, () => { + expect(() => + config.schema.validate({ + strict: false, + script_src: [`'self'`, `unsafe-inline`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + strict: false, + script_src: [`'self'`, `'unsafe-inline'`], + }) + ).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({ + script_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[script_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("throws if using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + script_src: [`hello`, `none`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[script_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + + expect(() => + config.schema.validate({ + script_src: [`hello`, `'none'`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[script_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + }); }); - 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\\""` - ); + 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({ + worker_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[worker_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("throws if using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + worker_src: [`hello`, `none`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[worker_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + + expect(() => + config.schema.validate({ + worker_src: [`hello`, `'none'`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[worker_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + }); }); - 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\\""` - ); + 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({ + style_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[style_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("throws if using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + style_src: [`hello`, `none`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[style_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + + expect(() => + config.schema.validate({ + style_src: [`hello`, `'none'`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[style_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); + }); + }); + + 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({ + connect_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[connect_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + connect_src: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + connect_src: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); + }); + + 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({ + default_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[default_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + default_src: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + default_src: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); + }); + + 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({ + font_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[font_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + font_src: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + font_src: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); + }); + + 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({ + frame_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + frame_src: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + frame_src: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); + }); + + 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({ + img_src: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[img_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + img_src: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + img_src: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); + }); + + 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({ + frame_ancestors: [`hello`, `nonce-foo`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_ancestors]: using \\"nonce-*\\" is considered insecure and is not allowed"` + ); + }); + it("allows using `none` or `'none'`", () => { + expect(() => + config.schema.validate({ + frame_ancestors: [`hello`, `none`], + }) + ).not.toThrow(); + + expect(() => + config.schema.validate({ + frame_ancestors: [`hello`, `'none'`], + }) + ).not.toThrow(); + }); }); }); diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index 533579d438fe59..d8d723ed941802 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -9,27 +9,81 @@ import { TypeOf, schema } from '@kbn/config-schema'; import { ServiceConfigDescriptor } from '../internal_types'; -const validateDirectiveValues = (values: string[]) => {}; +interface DirectiveValidationOptions { + allowNone: boolean; + allowNonce: boolean; +} -const cspDirectiveSchema = schema.arrayOf(schema.string(), { - defaultValue: [], - validate: validateDirectiveValues, -}); +const getDirectiveValidator = (options: DirectiveValidationOptions) => { + const validateValue = getDirectiveValueValidator(options); + return (values: string[]) => { + for (const value of values) { + const error = validateValue(value); + if (error) { + return error; + } + } + }; +}; + +const getDirectiveValueValidator = ({ allowNone, allowNonce }: DirectiveValidationOptions) => { + return (value: string) => { + if (!allowNonce && value.startsWith('nonce-')) { + return `using "nonce-*" is considered insecure and is not allowed`; + } + if (!allowNone && (value === `none` || value === `'none'`)) { + return `using "none" would conflict with Kibana's default csp configuration and is not allowed`; + } + }; +}; const configSchema = schema.object( { rules: schema.maybe(schema.arrayOf(schema.string())), - script_src: cspDirectiveSchema, - worker_src: cspDirectiveSchema, - style_src: cspDirectiveSchema, - connect_src: cspDirectiveSchema, - default_src: cspDirectiveSchema, - font_src: cspDirectiveSchema, - frame_src: cspDirectiveSchema, - img_src: cspDirectiveSchema, - frame_ancestors: cspDirectiveSchema, - report_uri: cspDirectiveSchema, - report_to: cspDirectiveSchema, + script_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), + }), + worker_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), + }), + style_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), + }), + connect_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + default_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + font_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + frame_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + img_src: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + frame_ancestors: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + report_uri: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), + report_to: schema.arrayOf(schema.string(), { + defaultValue: [], + validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + }), strict: schema.boolean({ defaultValue: true }), warnLegacyBrowsers: schema.boolean({ defaultValue: true }), disableEmbedding: schema.oneOf([schema.literal(false)], { defaultValue: false }), @@ -39,6 +93,13 @@ const configSchema = schema.object( 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"`; } + if ( + cspConfig.strict && + (cspConfig.script_src.includes(`unsafe-inline`) || + cspConfig.script_src.includes(`'unsafe-inline'`)) + ) { + return 'cannot use `unsafe-inline` for `script_src` when `csp.strict` is true'; + } }, } ); From ec383c39fa749941133b02aff4bf89df9b2e9a72 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Jun 2021 08:02:56 +0200 Subject: [PATCH 14/23] add additional settings to asciidoc --- docs/setup/settings.asciidoc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 39489dfd4e273f..e74162c1afdebd 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -51,6 +51,30 @@ that ship with {kib}. | `csp.style_src:` | Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src[Content Security Policy `style-src` directive]. +| `csp.connect_src:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/connect-src[Content Security Policy `connect-src` directive]. + +| `csp.default_src:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src[Content Security Policy `default-src` directive]. + +| `csp.font_src:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/font-src[Content Security Policy `font-src` directive]. + +| `csp.frame_src:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-src[Content Security Policy `frame-src` directive]. + +| `csp.img_src:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/img-src[Content Security Policy `img-src` directive]. + +| `csp.frame_ancestors:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors[Content Security Policy `frame-ancestors` directive]. + +| `csp.report_uri:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-uri[Content Security Policy `report-uri` directive]. + +| `csp.report_to:` +| Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-to[Content Security Policy `report-to` directive]. + |[[csp-strict]] `csp.strict:` | Blocks {kib} access to any browser that does not enforce even rudimentary CSP rules. In practice, this disables From 8a28a6261905a1a53a04368be6a724aa71872b90 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Jun 2021 08:05:25 +0200 Subject: [PATCH 15/23] add null-check --- src/core/server/csp/csp_directives.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index e60e49237ea14d..efb93868a9e3dc 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -97,37 +97,37 @@ const parseRules = (rules: string[]): Partial const parseConfigDirectives = (cspConfig: CspConfigType): Map => { const map = new Map(); - if (cspConfig.script_src.length) { + if (cspConfig.script_src?.length) { map.set('script-src', cspConfig.script_src); } - if (cspConfig.worker_src.length) { + if (cspConfig.worker_src?.length) { map.set('worker-src', cspConfig.worker_src); } - if (cspConfig.style_src.length) { + if (cspConfig.style_src?.length) { map.set('style-src', cspConfig.style_src); } - if (cspConfig.connect_src.length) { + if (cspConfig.connect_src?.length) { map.set('connect-src', cspConfig.connect_src); } - if (cspConfig.default_src.length) { + if (cspConfig.default_src?.length) { map.set('default-src', cspConfig.default_src); } - if (cspConfig.font_src.length) { + if (cspConfig.font_src?.length) { map.set('font-src', cspConfig.font_src); } - if (cspConfig.frame_src.length) { + if (cspConfig.frame_src?.length) { map.set('frame-src', cspConfig.frame_src); } - if (cspConfig.img_src.length) { + if (cspConfig.img_src?.length) { map.set('img-src', cspConfig.img_src); } - if (cspConfig.frame_ancestors.length) { + if (cspConfig.frame_ancestors?.length) { map.set('frame-ancestors', cspConfig.frame_ancestors); } - if (cspConfig.report_uri.length) { + if (cspConfig.report_uri?.length) { map.set('report-uri', cspConfig.report_uri); } - if (cspConfig.report_to.length) { + if (cspConfig.report_to?.length) { map.set('report-to', cspConfig.report_to); } From c74ada56ea7efa928e354f5d4ef938f7a752d340 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Jun 2021 08:07:11 +0200 Subject: [PATCH 16/23] revert test config props --- src/core/server/http/cookie_session_storage.test.ts | 6 +----- .../http/integration_tests/lifecycle_handlers.test.ts | 6 +----- src/core/server/http/test_utils.ts | 6 +----- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/core/server/http/cookie_session_storage.test.ts b/src/core/server/http/cookie_session_storage.test.ts index 3f67ee66995095..55af02a08561b5 100644 --- a/src/core/server/http/cookie_session_storage.test.ts +++ b/src/core/server/http/cookie_session_storage.test.ts @@ -16,7 +16,6 @@ import { CoreContext } from '../core_context'; import { HttpService } from './http_service'; import { KibanaRequest } from './router'; import { Env } from '../config'; -import type { CspConfigType } from '../csp'; import { contextServiceMock } from '../context/context_service.mock'; import { loggingSystemMock } from '../logging/logging_system.mock'; @@ -71,13 +70,10 @@ configService.atPath.mockImplementation((path) => { } if (path === 'csp') { return new BehaviorSubject({ - script_src: [], - worker_src: [], - style_src: [], strict: false, disableEmbedding: false, warnLegacyBrowsers: true, - } as CspConfigType); + }); } throw new Error(`Unexpected config path: ${path}`); }); diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts index 7d17afe95f9d4a..c2023c5577d617 100644 --- a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts +++ b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts @@ -11,7 +11,6 @@ import moment from 'moment'; import { BehaviorSubject } from 'rxjs'; import { ByteSizeValue } from '@kbn/config-schema'; -import type { CspConfigType } from '../../csp'; import { createHttpServer } from '../test_utils'; import { HttpService } from '../http_service'; import { HttpServerSetup } from '../http_server'; @@ -81,13 +80,10 @@ describe('core lifecycle handlers', () => { } if (path === 'csp') { return new BehaviorSubject({ - script_src: [], - worker_src: [], - style_src: [], strict: false, disableEmbedding: false, warnLegacyBrowsers: true, - } as CspConfigType); + }); } throw new Error(`Unexpected config path: ${path}`); }); diff --git a/src/core/server/http/test_utils.ts b/src/core/server/http/test_utils.ts index d39b2d62a4689e..4e1a88e967f8f1 100644 --- a/src/core/server/http/test_utils.ts +++ b/src/core/server/http/test_utils.ts @@ -15,7 +15,6 @@ import { HttpService } from './http_service'; import { CoreContext } from '../core_context'; import { getEnvOptions, configServiceMock } from '../config/mocks'; import { loggingSystemMock } from '../logging/logging_system.mock'; -import type { CspConfigType } from '../csp'; const coreId = Symbol('core'); const env = Env.createDefault(REPO_ROOT, getEnvOptions()); @@ -58,13 +57,10 @@ configService.atPath.mockImplementation((path) => { } if (path === 'csp') { return new BehaviorSubject({ - script_src: [], - worker_src: [], - style_src: [], strict: false, disableEmbedding: false, warnLegacyBrowsers: true, - } as CspConfigType); + }); } throw new Error(`Unexpected config path: ${path}`); }); From 59303f47f6bcc9ade4334397209b86cca14b9267 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Jun 2021 08:32:02 +0200 Subject: [PATCH 17/23] fix usage collection usage --- .../server/collectors/csp/csp_collector.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts b/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts index 6244fa561a4e3f..f5a783a6dfb05d 100644 --- a/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts +++ b/src/plugins/kibana_usage_collection/server/collectors/csp/csp_collector.test.ts @@ -27,6 +27,14 @@ describe('csp collector', () => { style_src: [], worker_src: [], script_src: [], + connect_src: [], + default_src: [], + font_src: [], + frame_src: [], + img_src: [], + frame_ancestors: [], + report_uri: [], + report_to: [], ...config, }); } From 36099819f992fec580fcbeef354de136115b0f26 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 24 Jun 2021 08:29:25 +0200 Subject: [PATCH 18/23] some review comments --- docs/setup/settings.asciidoc | 12 +++++ src/core/server/csp/config.test.ts | 60 +++++++++++++++------- src/core/server/csp/config.ts | 12 ++--- src/core/server/csp/csp_config.test.ts | 13 +++++ src/core/server/csp/csp_config.ts | 1 + src/core/server/csp/csp_directives.test.ts | 4 +- src/core/server/csp/csp_directives.ts | 6 +++ 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index e74162c1afdebd..ef9edd52cbc89e 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -69,6 +69,18 @@ that ship with {kib}. | `csp.frame_ancestors:` | Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors[Content Security Policy `frame-ancestors` directive]. +|=== + +[NOTE] +============ +The `frame-ancestors` directive can also be configured by using <>. In that case, that takes precedence and any values in `csp.frame_ancestors` +are ignored. +============ + +[cols="2*<"] +|=== + | `csp.report_uri:` | Add sources for the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-uri[Content Security Policy `report-uri` directive]. diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index fab83b4e8d8cc7..1b712860f67d73 100644 --- a/src/core/server/csp/config.test.ts +++ b/src/core/server/csp/config.test.ts @@ -210,18 +210,22 @@ describe('config.validate()', () => { `"[connect_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ connect_src: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[connect_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ connect_src: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[connect_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); @@ -250,18 +254,22 @@ describe('config.validate()', () => { `"[default_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ default_src: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[default_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ default_src: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[default_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); @@ -290,18 +298,22 @@ describe('config.validate()', () => { `"[font_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ font_src: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[font_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ font_src: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[font_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); @@ -330,18 +342,22 @@ describe('config.validate()', () => { `"[frame_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ frame_src: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ frame_src: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); @@ -370,18 +386,22 @@ describe('config.validate()', () => { `"[img_src]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ img_src: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[img_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ img_src: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[img_src]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); @@ -410,18 +430,22 @@ describe('config.validate()', () => { `"[frame_ancestors]: using \\"nonce-*\\" is considered insecure and is not allowed"` ); }); - it("allows using `none` or `'none'`", () => { + it("throws if using `none` or `'none'`", () => { expect(() => config.schema.validate({ frame_ancestors: [`hello`, `none`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_ancestors]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); expect(() => config.schema.validate({ frame_ancestors: [`hello`, `'none'`], }) - ).not.toThrow(); + ).toThrowErrorMatchingInlineSnapshot( + `"[frame_ancestors]: using \\"none\\" would conflict with Kibana's default csp configuration and is not allowed"` + ); }); }); }); diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index d8d723ed941802..f2075c942b7f63 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -54,27 +54,27 @@ const configSchema = schema.object( }), connect_src: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), default_src: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), font_src: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), frame_src: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), img_src: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), frame_ancestors: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), + validate: getDirectiveValidator({ allowNone: false, allowNonce: false }), }), report_uri: schema.arrayOf(schema.string(), { defaultValue: [], diff --git a/src/core/server/csp/csp_config.test.ts b/src/core/server/csp/csp_config.test.ts index 217d97979ac404..a1bac7d4ae73ee 100644 --- a/src/core/server/csp/csp_config.test.ts +++ b/src/core/server/csp/csp_config.test.ts @@ -157,6 +157,19 @@ describe('CspConfig', () => { 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, + disableEmbedding: true, + frame_ancestors: ['foo.com'], + }); + expect(config.disableEmbedding).toEqual(disableEmbedding); + expect(config.disableEmbedding).not.toEqual(CspConfig.DEFAULT.disableEmbedding); + expect(config.header).toMatchInlineSnapshot( + `"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'; frame-ancestors 'self'"` + ); + }); }); }); }); diff --git a/src/core/server/csp/csp_config.ts b/src/core/server/csp/csp_config.ts index 05e3d9edc7433d..13778088d9df2e 100644 --- a/src/core/server/csp/csp_config.ts +++ b/src/core/server/csp/csp_config.ts @@ -67,6 +67,7 @@ export class CspConfig implements ICspConfig { constructor(rawCspConfig: CspConfigType) { this.#directives = CspDirectives.fromConfig(rawCspConfig); if (!rawCspConfig.rules?.length && rawCspConfig.disableEmbedding) { + this.#directives.clearDirectiveValues('frame-ancestors'); this.#directives.addDirectiveValue('frame-ancestors', `'self'`); } diff --git a/src/core/server/csp/csp_directives.test.ts b/src/core/server/csp/csp_directives.test.ts index 36016804b6558c..1077b6ea9f3cd5 100644 --- a/src/core/server/csp/csp_directives.test.ts +++ b/src/core/server/csp/csp_directives.test.ts @@ -236,9 +236,9 @@ describe('CspDirectives', () => { "connect-src 'self' connect-src", "default-src 'self' default-src", "font-src 'self' font-src", - "frame-src frame-src", + "frame-src 'self' frame-src", "img-src 'self' img-src", - "frame-ancestors frame-ancestors", + "frame-ancestors 'self' frame-ancestors", "report-uri report-uri", "report-to report-to", ] diff --git a/src/core/server/csp/csp_directives.ts b/src/core/server/csp/csp_directives.ts index efb93868a9e3dc..9e3b60f7f1e4f7 100644 --- a/src/core/server/csp/csp_directives.ts +++ b/src/core/server/csp/csp_directives.ts @@ -39,6 +39,8 @@ export const additionalRules: Partial> = { 'default-src': [`'self'`], 'font-src': [`'self'`], 'img-src': [`'self'`], + 'frame-ancestors': [`'self'`], + 'frame-src': [`'self'`], }; export class CspDirectives { @@ -51,6 +53,10 @@ export class CspDirectives { this.directives.get(directiveName)!.add(normalizeDirectiveValue(directiveValue)); } + clearDirectiveValues(directiveName: CspDirectiveName) { + this.directives.delete(directiveName); + } + getCspHeader() { return this.getRules().join('; '); } From 966fc559187bbe6dd49856520139a141054cc9a3 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 24 Jun 2021 08:39:23 +0200 Subject: [PATCH 19/23] last review comments --- src/core/server/csp/config.test.ts | 24 +++++++++++++++++++++++- src/core/server/csp/config.ts | 25 +++++++++++++++---------- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index 1b712860f67d73..02dbca2c740f94 100644 --- a/src/core/server/csp/config.test.ts +++ b/src/core/server/csp/config.test.ts @@ -38,10 +38,31 @@ describe('config.validate()', () => { ); }); - it(`does not throw if containing 'unsafe-inline' when 'strict' is false`, () => { + it(`throws if containing 'unsafe-inline' when 'warnLegacyBrowsers' is true`, () => { + expect(() => + config.schema.validate({ + warnLegacyBrowsers: true, + script_src: [`'self'`, `unsafe-inline`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + ); + + expect(() => + config.schema.validate({ + warnLegacyBrowsers: true, + script_src: [`'self'`, `'unsafe-inline'`], + }) + ).toThrowErrorMatchingInlineSnapshot( + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + ); + }); + + it(`does not throw if containing 'unsafe-inline' when 'strict' and 'warnLegacyBrowsers' are false`, () => { expect(() => config.schema.validate({ strict: false, + warnLegacyBrowsers: false, script_src: [`'self'`, `unsafe-inline`], }) ).not.toThrow(); @@ -49,6 +70,7 @@ describe('config.validate()', () => { expect(() => config.schema.validate({ strict: false, + warnLegacyBrowsers: false, script_src: [`'self'`, `'unsafe-inline'`], }) ).not.toThrow(); diff --git a/src/core/server/csp/config.ts b/src/core/server/csp/config.ts index f2075c942b7f63..3a7cb20985cea3 100644 --- a/src/core/server/csp/config.ts +++ b/src/core/server/csp/config.ts @@ -82,7 +82,6 @@ const configSchema = schema.object( }), report_to: schema.arrayOf(schema.string(), { defaultValue: [], - validate: getDirectiveValidator({ allowNone: true, allowNonce: false }), }), strict: schema.boolean({ defaultValue: true }), warnLegacyBrowsers: schema.boolean({ defaultValue: true }), @@ -93,13 +92,16 @@ const configSchema = schema.object( 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"`; } - if ( - cspConfig.strict && - (cspConfig.script_src.includes(`unsafe-inline`) || - cspConfig.script_src.includes(`'unsafe-inline'`)) - ) { + const hasUnsafeInlineScriptSrc = + cspConfig.script_src.includes(`unsafe-inline`) || + cspConfig.script_src.includes(`'unsafe-inline'`); + + if (cspConfig.strict && hasUnsafeInlineScriptSrc) { return 'cannot use `unsafe-inline` for `script_src` when `csp.strict` is true'; } + if (cspConfig.warnLegacyBrowsers && hasUnsafeInlineScriptSrc) { + return 'cannot use `unsafe-inline` for `script_src` when `csp.warnLegacyBrowsers` is true'; + } }, } ); @@ -136,12 +138,15 @@ export const config: ServiceConfigDescriptor = { if (cspConfig?.rules) { addDeprecation({ message: - 'csp.rules is deprecated in favor of directive specific configuration. ' + - 'Please use `csp.script_src`, `csp.worker_src` and `csp.style_src` instead', + '`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.script_src", "csp.worker_src" and "csp.style_src"`, + `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".`, ], }, }); From 04a4f59257949c64a042bf8cb9847f4c520ce442 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 24 Jun 2021 08:40:56 +0200 Subject: [PATCH 20/23] add kibana-docker variables --- .../docker_generator/resources/base/bin/kibana-docker | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker index d109a824ca81de..b19f19377d023b 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker @@ -31,6 +31,17 @@ kibana_vars=( csp.rules csp.strict csp.warnLegacyBrowsers + csp.script_src + csp.worker_src + csp.style_src + csp.connect_src + csp.default_src + csp.font_src + csp.frame_src + csp.img_src + csp.frame_ancestors + csp.report_uri + csp.report_to data.autocomplete.valueSuggestions.terminateAfter data.autocomplete.valueSuggestions.timeout elasticsearch.customHeaders From e9590d475fc383b052b281938ba83b1b49217034 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 24 Jun 2021 09:09:10 +0200 Subject: [PATCH 21/23] try to fix doc reference --- docs/setup/settings.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index ef9edd52cbc89e..906f52a6ccca8b 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -73,8 +73,8 @@ that ship with {kib}. [NOTE] ============ -The `frame-ancestors` directive can also be configured by using <>. In that case, that takes precedence and any values in `csp.frame_ancestors` +The `frame-ancestors` directive can also be configured by using +<>. In that case, that takes precedence and any values in `csp.frame_ancestors` are ignored. ============ From fff64d86a7d4ee57c39ced0194edd7b6a7154ada Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Thu, 24 Jun 2021 09:59:29 +0200 Subject: [PATCH 22/23] try to fix doc reference again --- docs/setup/settings.asciidoc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/setup/settings.asciidoc b/docs/setup/settings.asciidoc index 906f52a6ccca8b..bcaa86d73adc4d 100644 --- a/docs/setup/settings.asciidoc +++ b/docs/setup/settings.asciidoc @@ -584,8 +584,7 @@ a|`server.securityResponseHeaders:` is used in all responses to the client from the {kib} server, and specifies what value is used. Allowed values are any text value or `null`. To disable, set to `null`. *Default:* `null` -[[server-securityResponseHeaders-disableEmbedding]] -a|`server.securityResponseHeaders:` +|[[server-securityResponseHeaders-disableEmbedding]]`server.securityResponseHeaders:` `disableEmbedding:` | 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 From 596db0d78a19d70ed9d8667acb49d9e0f8b3a1e4 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 25 Jun 2021 14:07:46 +0200 Subject: [PATCH 23/23] fix tests --- src/core/server/csp/config.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/server/csp/config.test.ts b/src/core/server/csp/config.test.ts index 02dbca2c740f94..6db93addb7da85 100644 --- a/src/core/server/csp/config.test.ts +++ b/src/core/server/csp/config.test.ts @@ -22,6 +22,7 @@ describe('config.validate()', () => { expect(() => config.schema.validate({ strict: true, + warnLegacyBrowsers: false, script_src: [`'self'`, `unsafe-inline`], }) ).toThrowErrorMatchingInlineSnapshot( @@ -31,6 +32,7 @@ describe('config.validate()', () => { expect(() => config.schema.validate({ strict: true, + warnLegacyBrowsers: false, script_src: [`'self'`, `'unsafe-inline'`], }) ).toThrowErrorMatchingInlineSnapshot( @@ -41,20 +43,22 @@ describe('config.validate()', () => { it(`throws if containing 'unsafe-inline' when 'warnLegacyBrowsers' is true`, () => { expect(() => config.schema.validate({ + strict: false, warnLegacyBrowsers: true, script_src: [`'self'`, `unsafe-inline`], }) ).toThrowErrorMatchingInlineSnapshot( - `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.warnLegacyBrowsers\` is true"` ); expect(() => config.schema.validate({ + strict: false, warnLegacyBrowsers: true, script_src: [`'self'`, `'unsafe-inline'`], }) ).toThrowErrorMatchingInlineSnapshot( - `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.strict\` is true"` + `"cannot use \`unsafe-inline\` for \`script_src\` when \`csp.warnLegacyBrowsers\` is true"` ); });