From ffe9a553f5dcb91fb3baa64e844854c0bf8719be Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Wed, 16 Nov 2022 21:45:10 +0100 Subject: [PATCH] Add CSP header to all requests, including api requests (#144902) Previously `/api/*` requests didn't include a `Content-Security-Policy` header, now they do. Closes #143871 (cherry picked from commit 5550ab6cb10fbfddf437a74900103bb33dd1afa4) # Conflicts: # src/core/server/http_resources/http_resources_service.test.ts # src/core/server/http_resources/http_resources_service.ts --- .../src/http_service.ts | 2 + .../src/lifecycle_handlers.test.ts | 21 ++++-- .../src/lifecycle_handlers.ts | 8 ++- .../core-http-server-mocks/src/test_utils.ts | 1 + .../http_resources_service.test.ts | 68 ++----------------- .../http_resources/http_resources_service.ts | 7 +- .../integration_tests/http/lifecycle.test.ts | 26 +++++++ 7 files changed, 61 insertions(+), 72 deletions(-) diff --git a/packages/core/http/core-http-server-internal/src/http_service.ts b/packages/core/http/core-http-server-internal/src/http_service.ts index 607413dcfd5464..45841b72bdbeeb 100644 --- a/packages/core/http/core-http-server-internal/src/http_service.ts +++ b/packages/core/http/core-http-server-internal/src/http_service.ts @@ -102,6 +102,8 @@ export class HttpService }, }); + registerCoreHandlers(prebootSetup, config, this.env); + if (this.shouldListen(config)) { this.log.debug('starting preboot server'); await this.prebootServer.start(); diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts index 6fa2ef18353b62..5e182005fd40c5 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.test.ts @@ -248,19 +248,28 @@ describe('customHeaders pre-response handler', () => { toolkit = createToolkit(); }); - it('adds the kbn-name header to the response', () => { - const config = createConfig({ name: 'my-server-name' }); + it('adds the kbn-name and Content-Security-Policy headers to the response', () => { + const config = createConfig({ + name: 'my-server-name', + csp: { strict: true, warnLegacyBrowsers: true, disableEmbedding: true, header: 'foo' }, + }); const handler = createCustomHeadersPreResponseHandler(config as HttpConfig); handler({} as any, {} as any, toolkit); expect(toolkit.next).toHaveBeenCalledTimes(1); - expect(toolkit.next).toHaveBeenCalledWith({ headers: { 'kbn-name': 'my-server-name' } }); + expect(toolkit.next).toHaveBeenCalledWith({ + headers: { + 'Content-Security-Policy': 'foo', + 'kbn-name': 'my-server-name', + }, + }); }); it('adds the security headers and custom headers defined in the configuration', () => { const config = createConfig({ name: 'my-server-name', + csp: { strict: true, warnLegacyBrowsers: true, disableEmbedding: true, header: 'foo' }, securityResponseHeaders: { headerA: 'value-A', headerB: 'value-B', // will be overridden by the custom response header below @@ -276,6 +285,7 @@ describe('customHeaders pre-response handler', () => { expect(toolkit.next).toHaveBeenCalledTimes(1); expect(toolkit.next).toHaveBeenCalledWith({ headers: { + 'Content-Security-Policy': 'foo', 'kbn-name': 'my-server-name', headerA: 'value-A', headerB: 'x', @@ -283,11 +293,13 @@ describe('customHeaders pre-response handler', () => { }); }); - it('preserve the kbn-name value from server.name if defined in custom headders ', () => { + it('do not allow overwrite of the kbn-name and Content-Security-Policy headers if defined in custom headders ', () => { const config = createConfig({ name: 'my-server-name', + csp: { strict: true, warnLegacyBrowsers: true, disableEmbedding: true, header: 'foo' }, customResponseHeaders: { 'kbn-name': 'custom-name', + 'Content-Security-Policy': 'custom-csp', headerA: 'value-A', headerB: 'value-B', }, @@ -300,6 +312,7 @@ describe('customHeaders pre-response handler', () => { expect(toolkit.next).toHaveBeenCalledWith({ headers: { 'kbn-name': 'my-server-name', + 'Content-Security-Policy': 'foo', headerA: 'value-A', headerB: 'value-B', }, diff --git a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts index 11e034a56914bc..3fe9c8ac727ffe 100644 --- a/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts +++ b/packages/core/http/core-http-server-internal/src/lifecycle_handlers.ts @@ -61,12 +61,18 @@ export const createVersionCheckPostAuthHandler = (kibanaVersion: string): OnPost }; export const createCustomHeadersPreResponseHandler = (config: HttpConfig): OnPreResponseHandler => { - const { name: serverName, securityResponseHeaders, customResponseHeaders } = config; + const { + name: serverName, + securityResponseHeaders, + customResponseHeaders, + csp: { header: cspHeader }, + } = config; return (request, response, toolkit) => { const additionalHeaders = { ...securityResponseHeaders, ...customResponseHeaders, + 'Content-Security-Policy': cspHeader, [KIBANA_NAME_HEADER]: serverName, }; diff --git a/packages/core/http/core-http-server-mocks/src/test_utils.ts b/packages/core/http/core-http-server-mocks/src/test_utils.ts index 2b9658693dce76..498020d294ba0f 100644 --- a/packages/core/http/core-http-server-mocks/src/test_utils.ts +++ b/packages/core/http/core-http-server-mocks/src/test_utils.ts @@ -26,6 +26,7 @@ const createConfigService = () => { configService.atPath.mockImplementation((path) => { if (path === 'server') { return new BehaviorSubject({ + name: 'kibana', hosts: ['localhost'], maxPayload: new ByteSizeValue(1024), autoListen: true, diff --git a/src/core/server/http_resources/http_resources_service.test.ts b/src/core/server/http_resources/http_resources_service.test.ts index 47b0ef049485b8..7c1615cd8cc244 100644 --- a/src/core/server/http_resources/http_resources_service.test.ts +++ b/src/core/server/http_resources/http_resources_service.test.ts @@ -55,6 +55,7 @@ describe('HttpResources service', () => { describe(`${name} register`, () => { const routeConfig: RouteConfig = { path: '/', validate: false }; let register: HttpResources['register']; + beforeEach(async () => { register = await initializer(); }); @@ -79,32 +80,8 @@ describe('HttpResources service', () => { } ); }); - - it('can attach headers, except the CSP header', async () => { - register(routeConfig, async (ctx, req, res) => { - return res.renderCoreApp({ - headers: { - 'content-security-policy': "script-src 'unsafe-eval'", - 'x-kibana': '42', - }, - }); - }); - - const [[, routeHandler]] = router.get.mock.calls; - - const responseFactory = httpResourcesMock.createResponseFactory(); - await routeHandler(context, kibanaRequest, responseFactory); - - expect(responseFactory.ok).toHaveBeenCalledWith({ - body: '', - headers: { - 'x-kibana': '42', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", - }, - }); - }); }); + describe('renderAnonymousCoreApp', () => { it('formats successful response', async () => { register(routeConfig, async (ctx, req, res) => { @@ -125,32 +102,8 @@ describe('HttpResources service', () => { } ); }); - - it('can attach headers, except the CSP header', async () => { - register(routeConfig, async (ctx, req, res) => { - return res.renderAnonymousCoreApp({ - headers: { - 'content-security-policy': "script-src 'unsafe-eval'", - 'x-kibana': '42', - }, - }); - }); - - const [[, routeHandler]] = router.get.mock.calls; - - const responseFactory = httpResourcesMock.createResponseFactory(); - await routeHandler(context, kibanaRequest, responseFactory); - - expect(responseFactory.ok).toHaveBeenCalledWith({ - body: '', - headers: { - 'x-kibana': '42', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", - }, - }); - }); }); + describe('renderHtml', () => { it('formats successful response', async () => { const htmlBody = ''; @@ -165,20 +118,17 @@ describe('HttpResources service', () => { body: htmlBody, headers: { 'content-type': 'text/html', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", }, }); }); - it('can attach headers, except the CSP & "content-type" headers', async () => { + it('can attach headers, except the "content-type" header', async () => { const htmlBody = ''; register(routeConfig, async (ctx, req, res) => { return res.renderHtml({ body: htmlBody, headers: { 'content-type': 'text/html5', - 'content-security-policy': "script-src 'unsafe-eval'", 'x-kibana': '42', }, }); @@ -194,12 +144,11 @@ describe('HttpResources service', () => { headers: { 'content-type': 'text/html', 'x-kibana': '42', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", }, }); }); }); + describe('renderJs', () => { it('formats successful response', async () => { const jsBody = 'alert(1);'; @@ -214,20 +163,17 @@ describe('HttpResources service', () => { body: jsBody, headers: { 'content-type': 'text/javascript', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", }, }); }); - it('can attach headers, except the CSP & "content-type" headers', async () => { + it('can attach headers, except the "content-type" header', async () => { const jsBody = 'alert(1);'; register(routeConfig, async (ctx, req, res) => { return res.renderJs({ body: jsBody, headers: { 'content-type': 'text/html', - 'content-security-policy': "script-src 'unsafe-eval'", 'x-kibana': '42', }, }); @@ -243,8 +189,6 @@ describe('HttpResources service', () => { headers: { 'content-type': 'text/javascript', 'x-kibana': '42', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", }, }); }); diff --git a/src/core/server/http_resources/http_resources_service.ts b/src/core/server/http_resources/http_resources_service.ts index 7e95dc9e302d9c..59767c12931b31 100644 --- a/src/core/server/http_resources/http_resources_service.ts +++ b/src/core/server/http_resources/http_resources_service.ts @@ -90,7 +90,6 @@ export class HttpResourcesService implements CoreService { }); }); +describe('runs with default preResponse handlers', () => { + it('does not allow overwriting of the "kbn-name" and "Content-Security-Policy" headers', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get({ path: '/', validate: false }, (context, req, res) => + res.ok({ + headers: { + foo: 'bar', + 'kbn-name': 'hijacked!', + 'Content-Security-Policy': 'hijacked!', + }, + }) + ); + await server.start(); + + const response = await supertest(innerServer.listener).get('/').expect(200); + + expect(response.header.foo).toBe('bar'); + expect(response.header['kbn-name']).toBe('kibana'); + expect(response.header['content-security-policy']).toBe( + `script-src 'self' 'unsafe-eval'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'` + ); + }); +}); + describe('run interceptors in the right order', () => { it('with Auth registered', async () => { const {