Skip to content

Commit

Permalink
[7.17] Add CSP header to all requests, including api requests (#144902)…
Browse files Browse the repository at this point in the history
… (#145553)

# Backport

This will backport the following commits from `main` to `7.17`:
- [Add CSP header to all requests, including api requests
(#144902)](#144902)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Thomas
Watson","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-11-16T20:45:10Z","message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:all-open","v8.6.0","v8.7.0"],"number":144902,"url":"https://github.com/elastic/kibana/pull/144902","mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/145449","number":145449,"state":"MERGED","mergeCommit":{"sha":"a9f7ba61f128e61beb936d2caff143e93e3321ea","message":"[8.6]
Add CSP header to all requests, including api requests (#144902)
(#145449)\n\n# Backport\n\nThis will backport the following commits from
`main` to `8.6`:\n- [Add CSP header to all requests, including api
requests\n(#144902)](https://github.com/elastic/kibana/pull/144902)\n\n<!---
Backport version: 8.9.7 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Thomas\nWatson\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2022-11-16T20:45:10Z\",\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\",\"branchLabelMapping\":{\"^v8.7.0$\":\"main\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:enhancement\",\"backport:all-open\",\"v8.7.0\"],\"number\":144902,\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v8.7.0\",\"labelRegex\":\"^v8.7.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/144902\",\"number\":144902,\"mergeCommit\":{\"message\":\"Add\nCSP
header to all requests, including api
requests\n(#144902)\\n\\nPreviously `/api/*` requests didn't include
a\n`Content-Security-Policy`\\r\\nheader, now they
do.\\r\\n\\r\\nCloses\n#143871\",\"sha\":\"5550ab6cb10fbfddf437a74900103bb33dd1afa4\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Thomas Watson
<[email protected]>"}},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144902","number":144902,"mergeCommit":{"message":"Add
CSP header to all requests, including api requests
(#144902)\n\nPreviously `/api/*` requests didn't include a
`Content-Security-Policy`\r\nheader, now they do.\r\n\r\nCloses
#143871","sha":"5550ab6cb10fbfddf437a74900103bb33dd1afa4"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
Thomas Watson and kibanamachine authored Nov 21, 2022
1 parent 7b7b3e3 commit 18c40db
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 72 deletions.
2 changes: 2 additions & 0 deletions src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export class HttpService
},
});

registerCoreHandlers(prebootSetup, config, this.env);

if (this.shouldListen(config)) {
this.log.debug('starting preboot server');
await this.prebootServer.start();
Expand Down
26 changes: 26 additions & 0 deletions src/core/server/http/integration_tests/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1453,6 +1453,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 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'`
);
});
});

describe('run interceptors in the right order', () => {
it('with Auth registered', async () => {
const {
Expand Down
39 changes: 35 additions & 4 deletions src/core/server/http/lifecycle_handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,19 +231,40 @@ describe('customHeaders pre-response handler', () => {
toolkit = httpServerMock.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: {
rules: [],
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: {
rules: [],
strict: true,
warnLegacyBrowsers: true,
disableEmbedding: true,
header: 'foo',
},
securityResponseHeaders: {
headerA: 'value-A',
headerB: 'value-B', // will be overridden by the custom response header below
Expand All @@ -259,18 +280,27 @@ 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',
},
});
});

it('preserve the kbn-name value from server.name if definied 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: {
rules: [],
strict: true,
warnLegacyBrowsers: true,
disableEmbedding: true,
header: 'foo',
},
customResponseHeaders: {
'kbn-name': 'custom-name',
'Content-Security-Policy': 'custom-csp',
headerA: 'value-A',
headerB: 'value-B',
},
Expand All @@ -283,6 +313,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',
},
Expand Down
8 changes: 7 additions & 1 deletion src/core/server/http/lifecycle_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,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,
};

Expand Down
1 change: 1 addition & 0 deletions src/core/server/http/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const configService = configServiceMock.create();
configService.atPath.mockImplementation((path) => {
if (path === 'server') {
return new BehaviorSubject({
name: 'kibana',
hosts: ['localhost'],
maxPayload: new ByteSizeValue(1024),
autoListen: true,
Expand Down
68 changes: 6 additions & 62 deletions src/core/server/http_resources/http_resources_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('HttpResources service', () => {
describe(`${name} register`, () => {
const routeConfig: RouteConfig<any, any, any, 'get'> = { path: '/', validate: false };
let register: HttpResources['register'];

beforeEach(async () => {
register = await initializer();
});
Expand All @@ -80,32 +81,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: '<body />',
headers: {
'x-kibana': '42',
'content-security-policy':
"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'",
},
});
});
});

describe('renderAnonymousCoreApp', () => {
it('formats successful response', async () => {
register(routeConfig, async (ctx, req, res) => {
Expand All @@ -126,32 +103,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: '<body />',
headers: {
'x-kibana': '42',
'content-security-policy':
"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'",
},
});
});
});

describe('renderHtml', () => {
it('formats successful response', async () => {
const htmlBody = '<html><body /></html>';
Expand All @@ -166,20 +119,17 @@ describe('HttpResources service', () => {
body: htmlBody,
headers: {
'content-type': 'text/html',
'content-security-policy':
"script-src 'unsafe-eval' '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 = '<html><body /></html>';
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',
},
});
Expand All @@ -195,12 +145,11 @@ describe('HttpResources service', () => {
headers: {
'content-type': 'text/html',
'x-kibana': '42',
'content-security-policy':
"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'",
},
});
});
});

describe('renderJs', () => {
it('formats successful response', async () => {
const jsBody = 'alert(1);';
Expand All @@ -215,20 +164,17 @@ describe('HttpResources service', () => {
body: jsBody,
headers: {
'content-type': 'text/javascript',
'content-security-policy':
"script-src 'unsafe-eval' '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',
},
});
Expand All @@ -244,8 +190,6 @@ describe('HttpResources service', () => {
headers: {
'content-type': 'text/javascript',
'x-kibana': '42',
'content-security-policy':
"script-src 'unsafe-eval' 'self'; worker-src blob: 'self'; style-src 'unsafe-inline' 'self'",
},
});
});
Expand Down
7 changes: 2 additions & 5 deletions src/core/server/http_resources/http_resources_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
request: KibanaRequest,
response: KibanaResponseFactory
): HttpResourcesServiceToolkit {
const cspHeader = deps.http.csp.header;
return {
async renderCoreApp(options: HttpResourcesRenderOptions = {}) {
const apmConfig = getApmConfig(request.url.pathname);
Expand All @@ -103,7 +102,7 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe

return response.ok({
body,
headers: { ...options.headers, 'content-security-policy': cspHeader },
headers: options.headers,
});
},
async renderAnonymousCoreApp(options: HttpResourcesRenderOptions = {}) {
Expand All @@ -118,7 +117,7 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe

return response.ok({
body,
headers: { ...options.headers, 'content-security-policy': cspHeader },
headers: options.headers,
});
},
renderHtml(options: HttpResourcesResponseOptions) {
Expand All @@ -127,7 +126,6 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
headers: {
...options.headers,
'content-type': 'text/html',
'content-security-policy': cspHeader,
},
});
},
Expand All @@ -137,7 +135,6 @@ export class HttpResourcesService implements CoreService<InternalHttpResourcesSe
headers: {
...options.headers,
'content-type': 'text/javascript',
'content-security-policy': cspHeader,
},
});
},
Expand Down

0 comments on commit 18c40db

Please sign in to comment.