Skip to content

Commit

Permalink
Further restrict CSP and fix misfiring Google Analytics violation (#532)
Browse files Browse the repository at this point in the history
- For all requests other than our HTML entrypoints and the playground worker script, serve a super strict policy,  just in case a response that shouldn't normally allow any code execution somehow actually does. Based on [this](w3c/webappsec#520 (comment)) and [this](webhintio/hint#3403 (comment)) comment.

- Add some directives that I found through https://csp-evaluator.withgoogle.com/ and [this comment](w3c/webappsec#520 (comment)).

- Temporary fix for inline Google Analytics script which was being reported as a violation. See #531 for details. Will fix properly in followup.
  • Loading branch information
aomarks authored Sep 30, 2021
1 parent fd603ad commit 91123b1
Showing 1 changed file with 62 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ export interface ContentSecurityPolicyMiddlewareOptions {
*/
const CSP_REPORT_URI = 'https://csp.withgoogle.com/csp/lit-dev';

/**
* TODO(aomarks) Generate this automatically. See
* https://github.com/lit/lit.dev/issues/531.
*/
const GOOGLE_ANALYTICS_INLINE_SCRIPT_HASH = `'sha256-bG+QS/Ob2lFyxJ7r7PCtj/a8YofLHFx4t55RzjR1znI='`;

/**
* Creates a Koa middleware that sets the lit.dev Content Security Policy (CSP)
* headers.
Expand All @@ -47,23 +53,40 @@ const CSP_REPORT_URI = 'https://csp.withgoogle.com/csp/lit-dev';
* https://www.w3.org/TR/CSP3/
* https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP
* https://speakerdeck.com/lweichselbaum/csp-a-successful-mess-between-hardening-and-mitigation
* https://csp-evaluator.withgoogle.com/
*/
export const contentSecurityPolicyMiddleware = (
opts: ContentSecurityPolicyMiddlewareOptions
): Koa.Middleware => {
const mainCsp = [
const makePolicy = (...directives: string[]) =>
[
...directives,
// Prevent an injected <base> tag from modifying relative URLs.
`base-uri 'none'`,
// Prevent form submissions.
`form-action 'none'`,
// Disallow other sites from iframing this site (e.g. clickjacking).
`frame-ancestors 'none'`,
...(opts.reportViolations ? [`report-uri ${CSP_REPORT_URI}`] : []),
].join('; ');

// Policy for the main HTML entrypoints (homepage, docs, playground, etc.)
const htmlCsp = makePolicy(
// TODO(aomarks) We should also enable trusted types, but that will require
// a policy in playground-elements for creating the worker, and a policy
// es-module-lexer for doing an eval (see next comment for more on that).

// TODO(aomarks) Remove unsafe-eval when https://crbug.com/1253267 is fixed.
// See comment below about playgroundWorkerCsp.
//
// In dev mode, data: scripts are required because @web/dev-server uses them
// for automatic reloads.
`script-src 'self' 'unsafe-eval' ${
opts.inlineScriptHashes?.map((hash) => `'${hash}'`).join(' ') ?? ''
} https://www.googletagmanager.com/gtag/js ${opts.devMode ? ` data:` : ''}`,
`script-src ${[
`'self'`,
// TODO(aomarks) Remove unsafe-eval when https://crbug.com/1253267 is fixed.
// See comment below about playgroundWorkerCsp.
`'unsafe-eval'`,
`https://www.googletagmanager.com/gtag/js`,
GOOGLE_ANALYTICS_INLINE_SCRIPT_HASH,
...(opts.inlineScriptHashes?.map((hash) => `'${hash}'`) ?? []),
// In dev mode, data: scripts are required because @web/dev-server uses them
// for automatic reloads.
...(opts.devMode ? [`data:`] : []),
].join(' ')}`,

// TODO(aomarks) Remove unpkg.com when https://crbug.com/1253267 is fixed.
// See comment below about playgroundWorkerCsp.
Expand Down Expand Up @@ -94,22 +117,27 @@ export const contentSecurityPolicyMiddleware = (
// The ytimg.com domain is needed for embedded YouTube videos.
`img-src 'self' data: https://i.ytimg.com/`,

// Disallow any embeds, applets, etc. This would usually be covered by
// `default-src: 'none'`, but we can't set that for the reason explained
// below.
`object-src 'none'`,

// TODO(aomarks) This could be 'none' if we didn't use <svg><use> elements,
// because Firefox does not follow the img-src directive for them, so there
// is no other directive to use. See
// https://bugzilla.mozilla.org/show_bug.cgi?id=1303364#c4 and
// https://github.com/w3c/webappsec-csp/issues/199.
`default-src 'self'`,

...(opts.reportViolations ? [`report-uri ${CSP_REPORT_URI}`] : []),
].join('; ');
`default-src 'self'`
);

// Policy for the playground-elements web worker script.
//
// TODO(aomarks) Currently this worker CSP will take effect in Firefox and
// Safari, but not Chrome. Chrome does not currently follow the CSP spec for
// workers; instead workers inherit the CSP policy of their parent context.
// This is being actively fixed (https://crbug.com/1253267), and once it ships
// we can remove unsafe-eval and unpkg.com from the main CSP above.
const playgroundWorkerCsp = [
const playgroundWorkerCsp = makePolicy(
// unsafe-eval is needed because we use es-module-lexer to parse import
// statements in modules. es-module-lexer needs unsafe-eval because:
//
Expand All @@ -136,18 +164,30 @@ export const contentSecurityPolicyMiddleware = (
`connect-src https://unpkg.com/`,

// Disallow everything else.
`default-src 'none'`,
...(opts.reportViolations ? [`report-uri ${CSP_REPORT_URI}`] : []),
].join('; ');
`default-src 'none'`
);

// For all other responses, set the strictest possible CSP, just in case a
// response that shouldn't normally allow any code execution actually does.
//
// See https://github.com/w3c/webappsec/issues/520#issuecomment-488516726 and
// https://github.com/webhintio/hint/issues/3403#issue-528402128 for
// discussion of why this is a good practice.
const strictFallbackCsp = makePolicy(`default-src 'none'`);

return async (ctx, next) => {
await next();

let policy: string;
if (ctx.response.type === 'text/html') {
// TODO(aomarks) Remove -Report-Only suffix when we are confident the
// policy is working.
ctx.set('Content-Security-Policy-Report-Only', mainCsp);
policy = htmlCsp;
} else if (ctx.path.endsWith('/playground-typescript-worker.js')) {
ctx.set('Content-Security-Policy-Report-Only', playgroundWorkerCsp);
policy = playgroundWorkerCsp;
} else {
policy = strictFallbackCsp;
}
// TODO(aomarks) Remove -Report-Only suffix when we are confident the
// policy is working.
ctx.set('Content-Security-Policy-Report-Only', policy);
};
};

0 comments on commit 91123b1

Please sign in to comment.