amp.dev icon indicating copy to clipboard operation
amp.dev copied to clipboard

Advise on how to tweak CSP for service worker

Open westonruter opened this issue 4 years ago • 9 comments
trafficstars

As pointed out by @jono-alderson, the CSP directive blocks a service worker from being installed. This adds a note to explain what needs to be changed in the CSP to accomodate a service worker.

westonruter avatar Mar 30 '21 21:03 westonruter

Oh hold on. It turns out that this is not even valid AMP!

image

In order for this to work in AMP, you have to send it as a Content-Security-Policy HTTP response header and not as a meta[http-equiv] tag.

This CSP example is also invalid AMP: https://github.com/ampproject/amphtml/blob/master/examples/csp.amp.html

westonruter avatar Apr 01 '21 04:04 westonruter

I also realize that the CSP will no longer work as-is with the new script[amp-onerror]. See https://github.com/ampproject/amphtml/issues/22543#issuecomment-673603872 and https://github.com/ampproject/amp-toolbox/issues/1146.

westonruter avatar Apr 01 '21 16:04 westonruter

@kristoferbaxter is there anyone on the eng team that would be able to give best practices for CSP?

patrickkettner avatar Apr 01 '21 17:04 patrickkettner

As for script[amp-onerror], the CSP is indicated in https://github.com/ampproject/amphtml/issues/22543#issuecomment-673603872.

westonruter avatar Apr 01 '21 19:04 westonruter

This PR LGTM, but I suggest some additional actions per the comments here.

To start, maybe update the above to what the Google AMP Cache outputs now for AMP documents:

$ curl -si https://amp-dev.cdn.ampproject.org/c/s/amp.dev/ | grep -i content-security-policy | cut -d\  -f2-
default-src * blob: data:; script-src 'sha256-5CxqAdDXlHviOy7zxeRpMobzRK/JNpLvkS+k8Zj3L3A=' 'sha256-FIBGC/wl1Qfnh2Fb5NPFHmRty7BHJdDpWW1FZ8egppI=' 'sha256-UXYprBCAtnqoL5acf14iemip/+HI+gDFh92yyXkM3XI=' 'sha256-dKn2nAtwgzaaXC8ZM58hhldxNyeuu4qrzW4H9//9YMA=' 'sha256-yAAlWuem9ue55JEvxkWhcWWA1Zu0p6cgbYtDWJjsdvs=' blob: https://cdn.ampproject.org/lts/ https://cdn.ampproject.org/rtv/ https://cdn.ampproject.org/sw/ https://cdn.ampproject.org/v0.js https://cdn.ampproject.org/v0.mjs https://cdn.ampproject.org/v0/ https://cdn.ampproject.org/viewer/; object-src 'none'; style-src 'unsafe-inline' https://cdn.ampproject.org/rtv/ https://cdn.materialdesignicons.com https://cloud.typography.com https://fast.fonts.net https://fonts.googleapis.com https://maxcdn.bootstrapcdn.com https://p.typekit.net https://pro.fontawesome.com https://use.fontawesome.com https://use.typekit.net; report-uri https://csp.withgoogle.com/csp/amp

This should cover the amp-onerror case. (I'm not sure what the other sha256's are about, off-hand.)

I also recommend a follow-up PR to fix https://github.com/ampproject/amphtml/blob/main/examples/csp.amp.html.

Further future:

  • Looks like updating these two docs should become part of the process for changes that require updating the CSP on the AMP Caches.
  • Maybe we should recommend a place to follow changes to the CSP that is required by optimized AMP. (It might change its value of amp-onerror one day?) This would also helpfully pick up any additions to the font allowlist that may occur for AMP (optimized or not).

twifkak avatar Apr 09 '21 18:04 twifkak

Also, maybe we should recommend that publishers replace report-uri with one that they can monitor.

twifkak avatar Apr 09 '21 19:04 twifkak

OK, looked into the various sha256s. I think all can be omitted except 5Cxq... and UXYp... which are for the onerror handler, pre-and-post mod/nomod, respectively.

twifkak avatar Apr 09 '21 19:04 twifkak

This PR LGTM, but I suggest some additional actions per the comments here.

Also, the page needs to be updated to advise on sending a Content-Security-Policy HTTP response header rather than a meta[http-equiv="Content-Security-Policy"] since the latter is invalid AMP, yeah?

westonruter avatar Apr 09 '21 21:04 westonruter

Also, the page needs to be updated to advise on sending a Content-Security-Policy HTTP response header rather than a meta[http-equiv="Content-Security-Policy"] since the latter is invalid AMP, yeah?

Oh oops, yeah.

twifkak avatar Apr 09 '21 23:04 twifkak