Skip to content

Commit

Permalink
Introduce content security policy (CSP) (#29545) (#30686)
Browse files Browse the repository at this point in the history
* csp: nonce and unsafe-eval for scripts

To kick things off, a rudimentary CSP implementation only allows
dynamically loading new JavaScript if it includes an associated nonce
that is generated on every load of the app.

A more sophisticated content security policy is necessary, particularly
one that bans eval for scripts, but one step at a time.

* img-src is not necessary if the goal is not to restrict

* configurable CSP owned by security team

* smoke test

* remove x-content-security-policy

* document csp.rules

* fix tsconfig for test

* switch integration test back to regular js

* stop looking for tsconfig in test

* grrr, linting errors not caught by precommit

* docs: people -> you for consistency sake

Co-Authored-By: epixa <court@epixa.com>
  • Loading branch information
epixa authored Feb 11, 2019
1 parent 72896ea commit 8461561
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 13 deletions.
2 changes: 2 additions & 0 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ you'll need to update your `kibana.yml` file. You can also enable SSL and set a

`cpuacct.cgroup.path.override:`:: Override for cgroup cpuacct path when mounted in manner that is inconsistent with `/proc/self/cgroup`

`csp.rules:`:: A template https://w3c.github.io/webappsec-csp/[content-security-policy] that disables certain unnecessary and potentially insecure capabilities in the browser. All instances of `{nonce}` will be replaced with an automatically generated nonce at load time. We strongly recommend that you keep the default CSP rules that ship with Kibana.

`elasticsearch.customHeaders:`:: *Default: `{}`* Header names and values to send to Elasticsearch. Any custom headers
cannot be overwritten by client-side headers, regardless of the `elasticsearch.requestHeadersWhitelist` configuration.

Expand Down
3 changes: 1 addition & 2 deletions packages/kbn-interpreter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"dependencies": {
"lodash": "npm:@elastic/lodash@3.10.1-kibana1",
"lodash.clone": "^4.5.0",
"scriptjs": "^2.5.8",
"uuid": "3.0.1"
},
"devDependencies": {
Expand All @@ -22,8 +21,8 @@
"babel-loader": "7.1.5",
"babel-plugin-transform-runtime": "^6.23.0",
"babel-polyfill": "6.20.0",
"css-loader": "1.0.0",
"copy-webpack-plugin": "^4.6.0",
"css-loader": "1.0.0",
"del": "^3.0.0",
"getopts": "^2.2.3",
"pegjs": "0.9.0",
Expand Down
16 changes: 14 additions & 2 deletions packages/kbn-interpreter/src/public/browser_registries.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,19 @@
* under the License.
*/

import $script from 'scriptjs';
function loadPath(path, callback) {
const script = document.createElement('script');

script.setAttribute('async', '');
script.setAttribute('nonce', window.__webpack_nonce__);
script.addEventListener('error', () => {
console.error('Failed to load plugin bundle', path);
});
script.setAttribute('src', path);
script.addEventListener('load', callback);

document.head.appendChild(script);
}

export const loadBrowserRegistries = (registries, basePath) => {
const remainingTypes = Object.keys(registries);
Expand All @@ -36,7 +48,7 @@ export const loadBrowserRegistries = (registries, basePath) => {
// Load plugins one at a time because each needs a different loader function
// $script will only load each of these once, we so can call this as many times as we need?
const pluginPath = `${basePath}/api/canvas/plugins?type=${type}`;
$script(pluginPath, () => {
loadPath(pluginPath, () => {
populatedTypes[type] = registries[type];
loadType();
});
Expand Down
13 changes: 11 additions & 2 deletions src/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ import Joi from 'joi';
import { constants as cryptoConstants } from 'crypto';
import os from 'os';

import { fromRoot } from '../../utils';
import { getData } from '../path';
import {
fromRoot
} from '../../utils';
import {
getData
} from '../path';
import { DEFAULT_CSP_RULES } from '../csp';

const tilemapSchema = Joi.object({
url: Joi.string(),
Expand Down Expand Up @@ -88,6 +93,10 @@ export default () => Joi.object({
exclusive: Joi.boolean().default(false)
}).default(),

csp: Joi.object({
rules: Joi.array().items(Joi.string()).default(DEFAULT_CSP_RULES),
}).default(),

cpu: Joi.object({
cgroup: Joi.object({
path: Joi.object({
Expand Down
72 changes: 72 additions & 0 deletions src/server/csp/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { createCSPRuleString, DEFAULT_CSP_RULES, generateCSPNonce } from './';

// 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,
// we test the default rules exactly so any change to those rules gets flagged
// for manual review. In otherwords, this test is intentionally fragile to draw
// extra attention if defaults are modified in any way.
//
// A test failure here does not necessarily mean this change cannot be made,
// but any change here should undergo sufficient scrutiny by the Kibana
// security team.
//
// The tests use inline snapshots to make it as easy as possible to identify
// the nature of a change in defaults during a PR review.
test('default CSP rules', () => {
expect(DEFAULT_CSP_RULES).toMatchInlineSnapshot(`
Array [
"script-src 'unsafe-eval' 'nonce-{nonce}'",
"worker-src blob:",
"child-src blob:",
]
`);
});

test('generateCSPNonce() creates a 16 character string', async () => {
const nonce = await generateCSPNonce();

expect(nonce).toHaveLength(16);
});

test('generateCSPNonce() creates a new string on each call', async () => {
const nonce1 = await generateCSPNonce();
const nonce2 = await generateCSPNonce();

expect(nonce1).not.toEqual(nonce2);
});

test('createCSPRuleString() converts an array of rules into a CSP header string', () => {
const csp = createCSPRuleString([`string-src 'self'`, 'worker-src blob:', 'img-src data: blob:']);

expect(csp).toMatchInlineSnapshot(`"string-src 'self'; worker-src blob:; img-src data: blob:"`);
});

test('createCSPRuleString() replaces all occurrences of {nonce} if provided', () => {
const csp = createCSPRuleString(
[`string-src 'self' 'nonce-{nonce}'`, 'img-src data: blob:', `default-src 'nonce-{nonce}'`],
'foo'
);

expect(csp).toMatchInlineSnapshot(
`"string-src 'self' 'nonce-foo'; img-src data: blob:; default-src 'nonce-foo'"`
);
});
41 changes: 41 additions & 0 deletions src/server/csp/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { randomBytes } from 'crypto';
import { promisify } from 'util';

const randomBytesAsync = promisify(randomBytes);

export const DEFAULT_CSP_RULES = Object.freeze([
`script-src 'unsafe-eval' 'nonce-{nonce}'`,
'worker-src blob:',
'child-src blob:',
]);

export async function generateCSPNonce() {
return (await randomBytesAsync(12)).toString('base64');
}

export function createCSPRuleString(rules: string[], nonce?: string) {
let ruleString = rules.join('; ');
if (nonce) {
ruleString = ruleString.replace(/\{nonce\}/g, nonce);
}
return ruleString;
}
1 change: 1 addition & 0 deletions src/ui/ui_render/bootstrap/template.js.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ window.onload = function () {
var dom = document.createElement('script');

dom.setAttribute('async', '');
dom.setAttribute('nonce', window.__webpack_nonce__);
dom.addEventListener('error', failure);
dom.setAttribute('src', file);
dom.addEventListener('load', next);
Expand Down
11 changes: 10 additions & 1 deletion src/ui/ui_render/ui_render_mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { i18n } from '@kbn/i18n';
import { AppBootstrap } from './bootstrap';
import { mergeVariables } from './lib';
import { fromRoot } from '../../utils';
import { generateCSPNonce, createCSPRuleString } from '../../server/csp';

export function uiRenderMixin(kbnServer, server, config) {
function replaceInjectedVars(request, injectedVars) {
Expand Down Expand Up @@ -176,7 +177,10 @@ export function uiRenderMixin(kbnServer, server, config) {
const request = h.request;
const basePath = request.getBasePath();

return h.view('ui_app', {
const nonce = await generateCSPNonce();

const response = h.view('ui_app', {
nonce,
uiPublicUrl: `${basePath}/ui`,
bootstrapScriptUrl: `${basePath}/bundles/app/${app.getId()}/bootstrap.js`,
i18n: (id, options) => i18n.translate(id, options),
Expand Down Expand Up @@ -206,6 +210,11 @@ export function uiRenderMixin(kbnServer, server, config) {
}),
},
});

const csp = createCSPRuleString(config.get('csp.rules'), nonce);
response.header('content-security-policy', csp);

return response;
}

server.decorate('toolkit', 'renderApp', function (app, injectedVarsOverrides) {
Expand Down
4 changes: 3 additions & 1 deletion src/ui/ui_render/views/ui_app.pug
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,6 @@ block content
.kibanaWelcomeText(data-error-message=i18n('common.ui.welcomeErrorMessage', { defaultMessage: 'Kibana did not load properly. Check the server output for more information.' }))
| #{i18n('common.ui.welcomeMessage', { defaultMessage: 'Loading Kibana' })}

script(src=bootstrapScriptUrl)
script(nonce=nonce).
window.__webpack_nonce__ = '!{nonce}';
script(src=bootstrapScriptUrl, nonce=nonce)
39 changes: 39 additions & 0 deletions test/api_integration/apis/general/csp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import expect from 'expect.js';

export default function ({ getService }) {
const supertest = getService('supertest');

describe('csp smoke test', () => {
it('app response sends content security policy headers', async () => {
const response = await supertest.get('/app/kibana');

expect(response.headers).to.have.property('content-security-policy');
});

it('csp header does not allow all inline scripts', async () => {
const response = await supertest.get('/app/kibana');

expect(response.headers['content-security-policy']).to.contain('script-src');
expect(response.headers['content-security-policy']).not.to.contain('unsafe-inline');
});
});
}
1 change: 1 addition & 0 deletions test/api_integration/apis/general/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
export default function ({ loadTestFile }) {
describe('general', () => {
loadTestFile(require.resolve('./cookies'));
loadTestFile(require.resolve('./csp'));
});
}
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -19222,11 +19222,6 @@ script-loader@0.7.2:
dependencies:
raw-loader "~0.5.1"

scriptjs@^2.5.8:
version "2.5.8"
resolved "https://registry.yarnpkg.com/scriptjs/-/scriptjs-2.5.8.tgz#d0c43955c2e6bad33b6e4edf7b53b8965aa7ca5f"
integrity sha1-0MQ5VcLmutM7bk7fe1O4llqnyl8=

scroll-into-view@^1.3.0:
version "1.9.1"
resolved "https://registry.yarnpkg.com/scroll-into-view/-/scroll-into-view-1.9.1.tgz#90c3b338422f9fddaebad90e6954790940dc9c1e"
Expand Down

0 comments on commit 8461561

Please sign in to comment.