Skip to content

Commit

Permalink
Replace CSP 'nonce-<base64>' directive with 'self' directive (#43553)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored Aug 21, 2019
1 parent 4540c55 commit 1331456
Show file tree
Hide file tree
Showing 19 changed files with 187 additions and 129 deletions.
6 changes: 2 additions & 4 deletions docs/setup/settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ in a 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.
certain unnecessary and potentially insecure capabilities in the browser. We
strongly recommend that you keep the default CSP rules that ship with Kibana.

`csp.strict:`:: *Default: `false`* Blocks access to Kibana to any browser that
does not enforce even rudimentary CSP rules. In practice, this will disable
Expand Down
3 changes: 0 additions & 3 deletions src/core/public/plugins/plugin_loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,12 @@ beforeEach(() => {
appendChildSpy = jest.spyOn(document.body, 'appendChild').mockReturnValue({} as any);

// Mock global fields needed for loading modules.
coreWindow.__kbnNonce__ = 'asdf';
coreWindow.__kbnBundles__ = {};
});

afterEach(() => {
appendChildSpy.mockRestore();
createElementSpy.mockRestore();
delete coreWindow.__kbnNonce__;
delete coreWindow.__kbnBundles__;
});

Expand All @@ -64,7 +62,6 @@ test('`loadPluginBundles` creates a script tag and loads initializer', async ()
'/bundles/plugin/plugin-a.bundle.js'
);
expect(fakeScriptTag.setAttribute).toHaveBeenCalledWith('id', 'kbn-plugin-plugin-a');
expect(fakeScriptTag.setAttribute).toHaveBeenCalledWith('nonce', 'asdf');
expect(fakeScriptTag.onload).toBeInstanceOf(Function);
expect(fakeScriptTag.onerror).toBeInstanceOf(Function);
expect(appendChildSpy).toHaveBeenCalledWith(fakeScriptTag);
Expand Down
4 changes: 0 additions & 4 deletions src/core/public/plugins/plugin_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export type UnknownPluginInitializer = PluginInitializer<unknown, Record<string,
* @internal
*/
export interface CoreWindow {
__kbnNonce__: string;
__kbnBundles__: {
[pluginBundleName: string]: UnknownPluginInitializer | undefined;
};
Expand Down Expand Up @@ -80,9 +79,6 @@ export const loadPluginBundle: LoadPluginBundle = <
script.setAttribute('id', `kbn-plugin-${pluginName}`);
script.setAttribute('async', '');

// Add kbnNonce for CSP
script.setAttribute('nonce', coreWindow.__kbnNonce__);

const cleanupTag = () => {
clearTimeout(timeout);
// Set to null for IE memory leak issue. Webpack does the same thing.
Expand Down
39 changes: 38 additions & 1 deletion src/legacy/server/config/transform_deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,42 @@ const dataPath = (settings, log) => {
}
};

const NONCE_STRING = `{nonce}`;
// Policies that should include the 'self' source
const SELF_POLICIES = Object.freeze(['script-src', 'style-src']);
const SELF_STRING = `'self'`;

const cspRules = (settings, log) => {
const rules = _.get(settings, 'csp.rules');
if (!rules) {
return;
}

const parsed = new Map(rules.map(ruleStr => {
const parts = ruleStr.split(/\s+/);
return [parts[0], parts.slice(1)];
}));

settings.csp.rules = [...parsed].map(([policy, sourceList]) => {
if (sourceList.find(source => source.includes(NONCE_STRING))) {
log(`csp.rules no longer supports the {nonce} syntax. Replacing with 'self' in ${policy}`);
sourceList = sourceList.filter(source => !source.includes(NONCE_STRING));

// Add 'self' if not present
if (!sourceList.find(source => source.includes(SELF_STRING))) {
sourceList.push(SELF_STRING);
}
}

if (SELF_POLICIES.includes(policy) && !sourceList.find(source => source.includes(SELF_STRING))) {
log(`csp.rules must contain the 'self' source. Automatically adding to ${policy}.`);
sourceList.push(SELF_STRING);
}

return `${policy} ${sourceList.join(' ')}`.trim();
});
};

const deprecations = [
//server
unused('server.xsrf.token'),
Expand All @@ -80,7 +116,8 @@ const deprecations = [
rewriteBasePath,
loggingTimezone,
configPath,
dataPath
dataPath,
cspRules
];

export const transformDeprecations = createTransform(deprecations);
136 changes: 127 additions & 9 deletions src/legacy/server/config/transform_deprecations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ import { transformDeprecations } from './transform_deprecations';

describe('server/config', function () {
describe('transformDeprecations', function () {

describe('savedObjects.indexCheckTimeout', () => {
it('removes the indexCheckTimeout and savedObjects properties', () => {
const settings = {
savedObjects: {
indexCheckTimeout: 123
}
indexCheckTimeout: 123,
},
};

expect(transformDeprecations(settings)).toEqual({});
Expand All @@ -38,22 +37,22 @@ describe('server/config', function () {
const settings = {
savedObjects: {
indexCheckTimeout: 123,
foo: 'bar'
}
foo: 'bar',
},
};

expect(transformDeprecations(settings)).toEqual({
savedObjects: {
foo: 'bar'
}
foo: 'bar',
},
});
});

it('logs that the setting is no longer necessary', () => {
const settings = {
savedObjects: {
indexCheckTimeout: 123
}
indexCheckTimeout: 123,
},
};

const log = sinon.spy();
Expand All @@ -62,5 +61,124 @@ describe('server/config', function () {
sinon.assert.calledWithExactly(log, sinon.match('savedObjects.indexCheckTimeout'));
});
});

describe('csp.rules', () => {
describe('with nonce source', () => {
it('logs a warning', () => {
const settings = {
csp: {
rules: [`script-src 'self' 'nonce-{nonce}'`],
},
};

const log = jest.fn();
transformDeprecations(settings, log);
expect(log.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"csp.rules no longer supports the {nonce} syntax. Replacing with 'self' in script-src",
],
]
`);
});

it('replaces a nonce', () => {
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'nonce-{nonce}'`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'unsafe-eval' 'nonce-{nonce}'`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'unsafe-eval' 'self'`]);
});

it('removes a quoted nonce', () => {
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'self' 'nonce-{nonce}'`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'nonce-{nonce}' 'self'`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
});

it('removes a non-quoted nonce', () => {
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'self' nonce-{nonce}`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
expect(
transformDeprecations(
{ csp: { rules: [`script-src nonce-{nonce} 'self'`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
});

it('removes a strange nonce', () => {
expect(
transformDeprecations(
{ csp: { rules: [`script-src 'self' blah-{nonce}-wow`] } },
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`]);
});

it('removes multiple nonces', () => {
expect(
transformDeprecations(
{
csp: {
rules: [
`script-src 'nonce-{nonce}' 'self' blah-{nonce}-wow`,
`style-src 'nonce-{nonce}' 'self'`,
],
},
},
jest.fn()
).csp.rules
).toEqual([`script-src 'self'`, `style-src 'self'`]);
});
});

describe('without self source', () => {
it('logs a warning', () => {
const log = jest.fn();
transformDeprecations({ csp: { rules: [`script-src 'unsafe-eval'`] } }, log);
expect(log.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"csp.rules must contain the 'self' source. Automatically adding to script-src.",
],
]
`);
});

it('adds self', () => {
expect(
transformDeprecations({ csp: { rules: [`script-src 'unsafe-eval'`] } }, jest.fn()).csp
.rules
).toEqual([`script-src 'unsafe-eval' 'self'`]);
});
});

it('does not add self to other policies', () => {
expect(
transformDeprecations({ csp: { rules: [`worker-src blob:`] } }, jest.fn()).csp.rules
).toEqual([`worker-src blob:`]);
});
});
});
});
27 changes: 1 addition & 26 deletions src/legacy/server/csp/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import {
createCSPRuleString,
generateCSPNonce,
DEFAULT_CSP_RULES,
DEFAULT_CSP_STRICT,
DEFAULT_CSP_WARN_LEGACY_BROWSERS,
Expand All @@ -40,7 +39,7 @@ import {
test('default CSP rules', () => {
expect(DEFAULT_CSP_RULES).toMatchInlineSnapshot(`
Array [
"script-src 'unsafe-eval' 'nonce-{nonce}'",
"script-src 'unsafe-eval' 'self'",
"worker-src blob:",
"child-src blob:",
"style-src 'unsafe-inline' 'self'",
Expand All @@ -56,32 +55,8 @@ test('CSP legacy browser warning defaults to enabled', () => {
expect(DEFAULT_CSP_WARN_LEGACY_BROWSERS).toBe(true);
});

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'"`
);
});
19 changes: 3 additions & 16 deletions src/legacy/server/csp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,8 @@
* 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}'`,
`script-src 'unsafe-eval' 'self'`,
'worker-src blob:',
'child-src blob:',
`style-src 'unsafe-inline' 'self'`,
Expand All @@ -33,14 +28,6 @@ export const DEFAULT_CSP_STRICT = false;

export const DEFAULT_CSP_WARN_LEGACY_BROWSERS = true;

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;
export function createCSPRuleString(rules: string[]) {
return rules.join('; ');
}
6 changes: 0 additions & 6 deletions src/legacy/ui/ui_bundles/app_entry_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ export const appEntryTemplate = (bundle) => `
* context: ${bundle.getContext()}
*/
// ensure the csp nonce is set in the dll
import 'dll/set_csp_nonce';
// set the csp nonce in the primary webpack bundle too
__webpack_nonce__ = window.__kbnNonce__;
// import global polyfills
import Symbol_observable from 'symbol-observable';
import '@babel/polyfill';
Expand Down
2 changes: 0 additions & 2 deletions src/legacy/ui/ui_render/bootstrap/template.js.hbs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var kbnCsp = JSON.parse(document.querySelector('kbn-csp').getAttribute('data'));
window.__kbnStrictCsp__ = kbnCsp.strictCsp;
window.__kbnNonce__ = kbnCsp.nonce;

if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) {
var legacyBrowserError = document.getElementById('kbn_legacy_browser_error');
Expand Down Expand Up @@ -65,7 +64,6 @@ if (window.__kbnStrictCsp__ && window.__kbnCspNotEnforced__) {
var dom = document.createElement('script');

dom.setAttribute('async', '');
dom.setAttribute('nonce', window.__kbnNonce__);
dom.addEventListener('error', failure);
dom.setAttribute('src', file);
dom.addEventListener('load', next);
Expand Down
Loading

0 comments on commit 1331456

Please sign in to comment.