From 5550ab6cb10fbfddf437a74900103bb33dd1afa4 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 --- .../src/http_resources_service.test.ts | 76 ++----------------- .../src/http_resources_service.ts | 8 +- .../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 + .../integration_tests/http/lifecycle.test.ts | 26 +++++++ 7 files changed, 63 insertions(+), 79 deletions(-) diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts index b642c505cad38..32245c6e9f61c 100644 --- a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts +++ b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.test.ts @@ -57,6 +57,7 @@ describe('HttpResources service', () => { describe(`${name} register`, () => { const routeConfig: RouteConfig = { path: '/', validate: false }; let register: HttpResources['register']; + beforeEach(async () => { register = await initializer(); }); @@ -81,32 +82,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 = createHttpResourcesResponseFactory(); - 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) => { @@ -127,32 +104,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 = createHttpResourcesResponseFactory(); - 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 = ''; @@ -167,20 +120,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', }, }); @@ -196,12 +146,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);'; @@ -216,20 +165,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', }, }); @@ -245,12 +191,11 @@ 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'", }, }); }); }); + describe('renderCss', () => { it('formats successful response', async () => { const cssBody = `body {border: 1px solid red;}`; @@ -265,20 +210,17 @@ describe('HttpResources service', () => { body: cssBody, headers: { 'content-type': 'text/css', - '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 cssBody = `body {border: 1px solid red;}`; register(routeConfig, async (ctx, req, res) => { return res.renderCss({ body: cssBody, headers: { 'content-type': 'text/css5', - 'content-security-policy': "script-src 'unsafe-eval'", 'x-kibana': '42', }, }); @@ -294,8 +236,6 @@ describe('HttpResources service', () => { headers: { 'content-type': 'text/css', 'x-kibana': '42', - 'content-security-policy': - "script-src 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'", }, }); }); diff --git a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts index 22be209158b89..13bd334148ae5 100644 --- a/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts +++ b/packages/core/http/core-http-resources-server-internal/src/http_resources_service.ts @@ -101,7 +101,6 @@ export class HttpResourcesService implements CoreService { 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 11e034a56914b..3fe9c8ac727ff 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 bb260ae23c908..18e6a21ed2dba 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/integration_tests/http/lifecycle.test.ts b/src/core/server/integration_tests/http/lifecycle.test.ts index 2833d9f0bad13..239350747c7fd 100644 --- a/src/core/server/integration_tests/http/lifecycle.test.ts +++ b/src/core/server/integration_tests/http/lifecycle.test.ts @@ -1473,6 +1473,32 @@ describe('OnPreResponse', () => { }); }); +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 {