From a37772f0f892a0334e728389f94040724064f655 Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Thu, 26 Jul 2018 15:48:28 +0200 Subject: [PATCH] fix(csp): remove duplicate sha-256 hashes (#3574) --- lib/core/middleware/nuxt.js | 65 +++++++++++++++++++++------------ lib/core/renderer.js | 10 ++--- test/unit/basic.ssr.csp.test.js | 55 ++++++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 33 deletions(-) diff --git a/lib/core/middleware/nuxt.js b/lib/core/middleware/nuxt.js index 57918d1243..35a4e68256 100644 --- a/lib/core/middleware/nuxt.js +++ b/lib/core/middleware/nuxt.js @@ -14,7 +14,7 @@ export default async function nuxtMiddleware(req, res, next) { await this.nuxt.callHook('render:route', req.url, result, context) const { html, - cspScriptSrcHashes, + cspScriptSrcHashSet, error, redirected, getPreloadFiles @@ -70,29 +70,7 @@ export default async function nuxtMiddleware(req, res, next) { if (this.options.render.csp) { const { allowedSources, policies } = this.options.render.csp - let cspStr = `script-src 'self'${this.options.dev ? " 'unsafe-eval'" : ''} ${(cspScriptSrcHashes).join(' ')}` - if (Array.isArray(allowedSources)) { - // For compatible section - cspStr += ' ' + allowedSources.join(' ') - } else if (typeof policies === 'object' && policies !== null && !Array.isArray(policies)) { - // Set default policy if necessary - if (!policies['script-src'] || !Array.isArray(policies['script-src'])) { - policies['script-src'] = [`'self'`].concat(cspScriptSrcHashes) - } else { - policies['script-src'] = cspScriptSrcHashes.concat(policies['script-src']) - if (!policies['script-src'].includes(`'self'`)) { - policies['script-src'] = [`'self'`].concat(policies['script-src']) - } - } - - // Make content-security-policy string - let cspArr = [] - Object.keys(policies).forEach((k) => { - cspArr.push(`${k} ${policies[k].join(' ')}`) - }) - cspStr = cspArr.join('; ') - } - res.setHeader('Content-Security-Policy', cspStr) + res.setHeader('Content-Security-Policy', getCspString({ cspScriptSrcHashSet, allowedSources, policies, isDev: this.options.dev })) } // Send response @@ -110,3 +88,42 @@ export default async function nuxtMiddleware(req, res, next) { next(err) } } + +const getCspString = ({ cspScriptSrcHashSet, allowedSources, policies, isDev }) => { + const joinedHashSet = Array.from(cspScriptSrcHashSet).join(' ') + const baseCspStr = `script-src 'self'${isDev ? ` 'unsafe-eval'` : ''} ${joinedHashSet}` + + if (Array.isArray(allowedSources)) { + return `${baseCspStr} ${allowedSources.join(' ')}` + } + + const policyObjectAvailable = typeof policies === 'object' && policies !== null && !Array.isArray(policies) + + if (policyObjectAvailable) { + const transformedPolicyObject = transformPolicyObject(policies, cspScriptSrcHashSet) + + return Object.entries(transformedPolicyObject).map(([k, v]) => `${k} ${v.join(' ')}`).join('; ') + } + + return baseCspStr +} + +const transformPolicyObject = (policies, cspScriptSrcHashSet) => { + const userHasDefinedScriptSrc = policies['script-src'] && Array.isArray(policies['script-src']) + + // Self is always needed for inline-scripts, so add it, no matter if the user specified script-src himself. + + const hashAndPolicySet = cspScriptSrcHashSet + hashAndPolicySet.add(`'self'`) + + if (!userHasDefinedScriptSrc) { + policies['script-src'] = Array.from(hashAndPolicySet) + return policies + } + + new Set(policies['script-src']).forEach(src => hashAndPolicySet.add(src)) + + policies['script-src'] = Array.from(hashAndPolicySet) + + return policies +} diff --git a/lib/core/renderer.js b/lib/core/renderer.js index 2a5cddbd0f..b0495a64d1 100644 --- a/lib/core/renderer.js +++ b/lib/core/renderer.js @@ -11,7 +11,7 @@ import connect from 'connect' import launchMiddleware from 'launch-editor-middleware' import consola from 'consola' -import { isUrl, waitFor, timeout } from '../common/utils' +import { isUrl, timeout, waitFor } from '../common/utils' import defaults from '../common/nuxt.config' import MetaRenderer from './meta' @@ -369,14 +369,12 @@ export default class Renderer { isJSON: true })};` - const cspScriptSrcHashes = [] + const cspScriptSrcHashSet = new Set() if (this.options.render.csp) { const { hashAlgorithm } = this.options.render.csp let hash = crypto.createHash(hashAlgorithm) hash.update(serializedSession) - cspScriptSrcHashes.push( - `'${hashAlgorithm}-${hash.digest('base64')}'` - ) + cspScriptSrcHashSet.add(`'${hashAlgorithm}-${hash.digest('base64')}'`) } APP += `` @@ -396,7 +394,7 @@ export default class Renderer { return { html, - cspScriptSrcHashes, + cspScriptSrcHashSet, getPreloadFiles: context.getPreloadFiles, error: context.nuxt.error, redirected: context.redirected diff --git a/test/unit/basic.ssr.csp.test.js b/test/unit/basic.ssr.csp.test.js index b1d08de610..4d8dc5287d 100644 --- a/test/unit/basic.ssr.csp.test.js +++ b/test/unit/basic.ssr.csp.test.js @@ -1,4 +1,4 @@ -import { loadFixture, getPort, Nuxt, rp } from '../utils' +import { getPort, loadFixture, Nuxt, rp } from '../utils' let port const url = route => 'http://localhost:' + port + route @@ -40,6 +40,23 @@ describe('basic ssr csp', () => { } ) + test( + 'Contain only unique hashes in header when csp is set', + async () => { + const nuxt = await startCSPTestServer(true) + const { headers } = await rp(url('/stateless'), { + resolveWithFullResponse: true + }) + + const hashes = headers['content-security-policy'].split(' ').filter(s => s.startsWith('\'sha256-')) + const uniqueHashes = [...new Set(hashes)] + + expect(uniqueHashes.length).toBe(hashes.length) + + await nuxt.close() + } + ) + test( 'Contain Content-Security-Policy header, when csp.allowedSources set', async () => { @@ -77,7 +94,7 @@ describe('basic ssr csp', () => { }) expect(headers['content-security-policy']).toMatch(/default-src 'none'/) - expect(headers['content-security-policy']).toMatch(/script-src 'self' 'sha256-.*'/) + expect(headers['content-security-policy']).toMatch(/script-src 'sha256-(.*)?' 'self'/) expect(headers['content-security-policy'].includes('https://example.com')).toBe(true) expect(headers['content-security-policy'].includes('https://example.io')).toBe(true) @@ -101,7 +118,39 @@ describe('basic ssr csp', () => { }) expect(headers['content-security-policy']).toMatch(/default-src 'none'/) - expect(headers['content-security-policy']).toMatch(/script-src 'self' 'sha256-.*'/) + expect(headers['content-security-policy']).toMatch(/script-src 'sha256-.*' 'self'$/) + + await nuxt.close() + } + ) + + test( + 'Contain only unique hashes in header when csp.policies is set', + async () => { + const policies = { + 'default-src': [`'self'`], + 'script-src': [`'self'`], + 'style-src': [`'self'`] + } + + const nuxt = await startCSPTestServer({ + policies + }) + + for (let i = 0; i < 5; i++) { + await rp(url('/stateless'), { + resolveWithFullResponse: true + }) + } + + const { headers } = await rp(url('/stateful'), { + resolveWithFullResponse: true + }) + + const hashes = headers['content-security-policy'].split(' ').filter(s => s.startsWith('\'sha256-')) + const uniqueHashes = [...new Set(hashes)] + + expect(uniqueHashes.length).toBe(hashes.length) await nuxt.close() }