[WIP] Strong CSP Support #4943

dav-is · 2018-08-10T17:21:27Z

Support for Content Security Policy requires testing Next.js with the strictest policy possible. This is to support the use of a strict policy on hardened Next.js application. To be compatible, Next.js must not use any inline styles or scripts. No dependencies can use eval(). This is all to help prevent XSS attacks.

With recent reports of malicious packages on npm, users can benefit from whitelisting the origins they communicate with avoid leaking private data (unintentionally or maliciously).

TODO

New Configuration Options

// next.config.js
module.exports = {
  //This is the same policy we test against internally
  contentSecurityPolicy: "default-src 'none'; script-src 'self'; style-src 'nonce-{style-nonce}'; connect-src 'self';img-src 'self';"
}

Server Side Rendering State is stored inside a <script type="application/json"> which is not executed so is not considered an inline script.

If using static exporting, we can't set a CSP in a header, so the policy will be added in a meta tag. If using static exporting nonces cannot work, so either 'unsafe-inline' is required for styles or you will have to use a different CSS-in-JS library that can export statically as well. It would be cool if we could output a now.json file to add a CSP header to static hosting.

dbo · 2018-08-11T07:13:49Z

server/document.js

-      }} />}
+      {staticMarkup ? null : <script
+        id="server-app-state"
+        type="application/json"


Can't you use another tag type instead for transferring the app-state? I suppose even though its type is application/json some browsers may treat it as an inline script.

No. This is an html standard. <script type=“application/json”> isn’t JavaScript and CSP defines script-src as “Specifies valid sources for JavaScript”. This is also the most valid way for SSR to use CSP.

I wouldn't be so optimistic about all browsers implementing this correctly. Anyway, alternatively I could think about using a data- attribute on the head element to hold the app-state.

https://www.drupal.org/node/2513818

If it means anything to you, a quick google search found that Drupal is using this method.

dbo · 2018-08-11T07:13:56Z

server/document.js

+          ),
+        }}
+      />}
+      <script


This defeats the purpose of this PR, defines an inline script.

As stated above, CSP ignores non-JavaScript <script> tags. Using a <script> tag is the html standard of transmitting JSON as well.

But I don't see any type attribute set on this script tag, so it's treated as javascript. Have you been testing your PR with a strict CSP?

Oh I just realized which script tag this comment was attached to. This script tag is temporary until I can figure out how to define this through webpack (this is why it’s a WIP)

dbo · 2018-08-11T07:14:05Z

server/document.js

+        }}
+      />
+      <script
+        src={`${assetPrefix}/_next/static/runtime/bootstrap.js`}


I'd like to see this discussed upfront, since this adds a network request. I'd rather prefer to have the app-state/bootstrap code integrated into some common bundle (_app?).

It seems like all the pages are loaded asyncronously so they could be loaded before runtime/main.js which is where next is loaded. I added a new file called runtime/bootstrap.js that should be small and won’t be loaded asyncronously and will always run before everything else. This is a potential blocking process, but if we keep the file small and never change it so we can keep a huge cache time.

The app-state is data (nothing that is executed) that will change with each request to SSR, but the bootstrap.js can be cached instead of being sent with each html request so it’s actually going to save bytes over time, and if you are using http/2 you can have more requests without any performance degradation.

dav-is · 2018-08-28T03:32:39Z

This is ready for review! 😄

CC: @dbo

dbo · 2018-08-28T06:32:01Z

README.md

+
+#### Content Security Policy
+
+Next.js *mostly* supports [CSP](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP). You can use the following CSP policy to get started: `default-src 'self'; style-src 'self' 'unsafe-inline';`


I think unsafe-inline is a bad suggestion for CSP and not needed, since you could define a hash for the inline script (NextScript).

This is inline style. Completely different issue. Trust me. <style> blocks are just randomly added and removed from the DOM. It takes some special logic to get that working.

I’m going to see if I can get automatic CSP support in Nexjs today including nonces for generates styles.

dbo · 2018-08-28T06:35:08Z

test/integration/app-document/pages/_document.js

-        <Head nonce='test-nonce'>
-          {csp ? <meta httpEquiv='Content-Security-Policy' content={csp} /> : null}
+        <Head>
+          {this.props.withCSP ? <meta httpEquiv='Content-Security-Policy' content="default-src 'self'; style-src 'self' 'unsafe-inline';" /> : null}


This is actually weakening the current CSP of the test. Why are you suggesting this? It does not make sense to me, users should rather define a hash in their CSP for the exceptional inline scripts they need, or nonces (if they run a server to reliably generate fresh nones, of course).

Not true at all. Look closely at your CSP policy when you were using hashes: https://github.com/zeit/next.js/blob/canary/test/integration/app-document/pages/_document.js#L38

Inline styles is a different issue and a much more minor issue as it’s only a tiny visual security hole (instead of something being executed).

You're right, I misread that rule.

timneutkens · 2018-08-28T08:52:01Z

client/bootstrap.js

@@ -0,0 +1,17 @@
+window.module = {}
+window.__NEXT_DATA__ = JSON.parse(


This can be done in client/index.js right 🤔

No. There’s a function appended to the beginning of each page.js and they load async so they could be called at any moment. This means that these functions need to be defined before any other scripts are loaded, which is only possible by loading it first as the client/index.js could be loaded third and two pages have loaded first and there’s no function to register them.

Also, module={} has to be defined before any webpack files load as they begin with module.exports and will say module is undefined. I’m guessing these are all the hoops you had to jump through to get asynchronous loading working.

window.__NEXT_DATA__ = JSON.parse(

I meant reading the __NEXT_DATA__, the rest still has to be defined beforehand like you said.

@timneutkens Becuase __NEXT_DATA__ is used in the lines following: https://github.com/zeit/next.js/pull/4943/files#diff-358a769020e11c021478d8f344f90a7eR11

timneutkens · 2018-08-28T08:53:44Z

client/bootstrap.js

@@ -0,0 +1,17 @@
+window.module = {}


I wonder if this is still needed with the latest webpack, it might not be needed anymore

Removing it breaks everything.

Fixed here: #5093

dbo · 2018-08-28T13:03:19Z

server/document.js

-      {page !== '/_error' && <link rel='preload' href={`${assetPrefix}/_next/static/${buildId}/pages${pagePathname}`} as='script' nonce={this.props.nonce} />}
-      <link rel='preload' href={`${assetPrefix}/_next/static/${buildId}/pages/_app.js`} as='script' nonce={this.props.nonce} />
-      <link rel='preload' href={`${assetPrefix}/_next/static/${buildId}/pages/_error.js`} as='script' nonce={this.props.nonce} />
+      <link rel='preload' href={`${assetPrefix}/_next/static/runtime/bootstrap.js`} as='script' />


Is there an agreement about a further script request, or can the bootstrap code be integrated into a bundle?

There's no guarantee about execution order of scripts, so it would be hard to ensure that the functions in bootstrap.js are run first. I looked into adding it into the webpack runtime, but it seemed more complicated than it was worth.

examples/with-strict-csp/server.js

timneutkens · 2018-08-28T17:09:53Z

server/document.js

  render () {
    const { staticMarkup, assetPrefix, devFiles, __NEXT_DATA__ } = this.context._documentProps
    const { page, pathname, buildId } = __NEXT_DATA__
    const pagePathname = getPagePathname(pathname)

+    __NEXT_DATA__.cleanPathname = htmlescape(__NEXT_DATA__.pathname);


What is the possible attack vector here, assuming that's the reason you're escaping the pathname 🤔 pathname is already part of __NEXT_DATA__, and __NEXT_DATA__ gets htmlescaped too

I didn’t want to have to bundle htmlescape into the the bootstrap file. I just escape the value early: https://github.com/zeit/next.js/blob/canary/server/document.js#L192

dav-is · 2018-12-11T01:20:45Z

Turns out that CSP doesn't like

<iframe>
  <script>alert('test')</script>
</iframe>

so react-error-overlay doesn't work without having script-src 'unsafe-inline'. We would need to create a PR that uses <iframe src="/error_overlay"></iframe> which I'm not sure how feasible it would be... maybe it's time to implement a custom Next.js error overlay? 😞

Also, we still have a few tests and error pages that use inline styles.

timneutkens · 2018-12-11T10:10:59Z

cc @Timer @iansu

Timer · 2018-12-11T15:29:28Z

We're more than happy to implement any changes needed to make the overlay CSP compatible.

I'm not a big fan of facebook/create-react-app#5943 because it looks like the user mounting the overlay will have to be aware of what styles to apply.

How do libraries like styled-components do this in a client-side friendly way (or is this behavior special to iframe)? Can we just make the default behavior CSP friendly for everyone?

We've been slow to adopt CSP because the overlay is mainly a development tool, not a production addon. I'm open to any proposals.

Can we still do things like this?
https://github.com/facebook/create-react-app/blob/8dc1902f2f655e26fe86647ea2cb444db69dd694/packages/react-error-overlay/src/utils/dom/absolutifyCaret.js

dav-is · 2018-12-11T15:40:10Z

@Timer The root iframe has styles applied to it inline. The PR allows us to move the styles into <style nonce=""> react-error-overlay isnt scoped to do something like this and the styles are just setting top: 0 and basic things like that that don't change.

The problem with using an iframe is that you can't write everything inline. You'd need to create a new page to view in an iframe and use some kind of API to communicate between the iframe and the runtime error. Google does something kind of similar with Auth although I don't know how they do it...

Timer · 2018-12-11T15:45:14Z

Are pages allowed to inject their own new style tags? Can we create a new style tag in the current page and then assign it to the iframe using loadingIframe.className = "";?

dav-is · 2018-12-11T17:38:09Z

In order to have safe CSP styles all styles need to either be in a stylesheet externally (which could be done inside of react-error-overlay) or internally with a nonce (<style nonce="secretThatAppHandles">). So an improvement of my PR for react error overlay could serve a stylesheet just for the root iframe element. That would solve the unsafe-inline of styles.

The unsafe-inline of scripts is a bit more serious and difficult to solve. I'm diving deeper into how iframes work now

dav-is · 2018-12-11T20:28:35Z

@Timer I wrote a small html page to demonstrate what I mean. It's sort of a complicated setup division that makes me question if we should even be using an iframe for this...

Code: https://github.com/dav-is/csp-iframe-example
Live Code: https://dav-is.github.io/csp-iframe-example/

Also, you meantioned if you can still do this and you can by using c.className = c.className + ' absolute'; and defining that change in your CSS file.

The benefit of having a strict CSP during development is so you can catch issues before testing the production code. You could build up a concept that doesn't work with your policy (that could be determined and enforced as a team) and you realize it doesn't work with the policy after you finished and ran tests. Or maybe someone doesn't have reliable production build tests and a CSP bug slips through because it worked in development mode.

Jero786 · 2019-01-11T10:38:00Z

There is some update from this PR? there would be awesome to be in the next release version.

timneutkens · 2019-01-11T11:01:20Z

This is not going to land before v8. Will be worked on at a later time.

baldurh · 2019-02-19T07:38:43Z

I’m happy to see there’s progress on this issue 🙂 Before I dive in and try it myself, is there a way for me to manually add CSP to my application or is there still some issues in v8 that prevent that? In the v8 release blog post they mention that before v8 it was only possible using script-src 'unsafe-inline' but now you should be able to use script-src 'self'. Is there anything else I can expect to stop me from manually adding the headers to my responses?

dav-is · 2019-02-19T14:20:04Z

@baldurh CSP works in production mode, but you'll run into some errors in development, so you'll need to test your CSP on your production build for now. There's also support for style-src in this PR

timneutkens · 2019-05-29T11:02:11Z

Going to close this PR as it's outdated. We'll tackle it later on 👌 @dav-is has since joined the Next.js team 👍

monokrome · 2020-06-29T23:00:38Z

@timneutkens Hi! Can you link the follow-up PR for those of us who are following along from past issues here? I'm wondering what the status of CSP in Next is, but this seems like the end of the trail for people who came from the previous issue right now 🙀

ghost · 2020-07-06T07:34:45Z

any new? documentation?

timneutkens · 2020-07-06T08:26:02Z

Here's the example: https://github.com/vercel/next.js/tree/canary/examples/with-strict-csp. You can search the releases for the related PRs that removed inline JS.

javidjamae · 2020-08-25T04:26:47Z

@conioX Looks like you can include Content Security Policy headers in NextJS 9.5

https://nextjs.org/blog/next-9-5#headers
https://nextjs.org/docs/api-reference/next.config.js/headers

rogerweb · 2020-08-27T18:41:40Z

The last item in the above TODO list reads

"Add nonce support to other CSS-in-JS libraries with updated examples"

and the provided example only mentions

Note: There are still valid cases for using a nonce in case you need to inline scripts or styles for which calculating a hash is not feasible.

Is this something that we can expect in the near future? I would appreciate it very much as I'm struggling to make CSP to work with Material-UI.

dav-is added 2 commits August 10, 2018 12:12

WIP Fix

386245b

Merge branch 'canary' into canary

1223ded

dav-is mentioned this pull request Aug 10, 2018

Content Security Policy (CSP) with next.js #256

Closed

dav-is added 3 commits August 10, 2018 13:47

Generate bootstrap.js

23012bc

Syntax

e5c1a51

Merge branch 'canary' of github.com:dav-is/next.js into canary

e18b1ad

dbo reviewed Aug 11, 2018

View reviewed changes

dav-is added 7 commits August 27, 2018 17:20

Merge branch 'canary' into canary

9531f55

Copy bootstrap.js instead of loading through webpack

b840984

Cleanup

9ec8c96

add a htmlescaped pathname

709fc5c

Fix Flow Error

7e7092f

Remove hashes and nonces

986adfb

Minor fixes

9e7ee16

dav-is changed the title ~~WIP: Move inline scripts elsewhere for strong CSP support~~ Move Inline Script to Bootstraping File to Support Strong CSP Aug 28, 2018

Remove mentions of nonce/hash, add CSP to README.md

f3eddab

dbo reviewed Aug 28, 2018

View reviewed changes

timneutkens reviewed Aug 28, 2018

View reviewed changes

dbo reviewed Aug 28, 2018

View reviewed changes

examples/with-strict-csp/server.js Show resolved Hide resolved

Merge branch 'canary' into canary

2c76ed9

timneutkens reviewed Aug 28, 2018

View reviewed changes

dav-is added 2 commits August 28, 2018 12:52

Add Strict CSP Support//waiting for styled-jsx PR Merge

178e4fe

Merge branch 'canary' of github.com:dav-is/next.js into canary

6e02af3

dav-is changed the title ~~Move Inline Script to Bootstraping File to Support Strong CSP~~ WIP: Move Inline Script to Bootstraping File to Support Strong CSP Aug 28, 2018

dav-is added 2 commits December 10, 2018 18:57

Fix ETags and remove unused dep

15cf4f4

Merge branch 'canary' of github.com:dav-is/next.js into canary

bbd7306

dav-is added 6 commits December 12, 2018 14:15

Merge branch 'canary' into canary

eaa0672

Readme edits

8fea414

Merge branch 'canary' of github.com:dav-is/next.js into canary

3608c1a

Remove crossOrigin PR

abfaa78

Merge remote-tracking branch 'upstream/canary' into canary

247aa1d

Remove inline styles from _error.js and error-debug.js

cf30cb6

timneutkens changed the title ~~Strong CSP Support~~ [WIP] Strong CSP Support Dec 20, 2018

timneutkens closed this May 29, 2019

rgreinho mentioned this pull request Jan 29, 2020

Leverage Mozilla Observatory to improve security scrapd/scrapdviz#194

Open

overlookmotel mentioned this pull request May 3, 2020

Move data cache into <script type="application/json"> tag overlookmotel/react-lazy-data#19

Open

vercel locked as resolved and limited conversation to collaborators Jan 30, 2022


		#### Content Security Policy

		Next.js mostly supports [CSP](https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP). You can use the following CSP policy to get started: `default-src 'self'; style-src 'self' 'unsafe-inline';`

		@@ -0,0 +1,17 @@
		window.module = {}
		window.__NEXT_DATA__ = JSON.parse(

[WIP] Strong CSP Support #4943

[WIP] Strong CSP Support #4943

Conversation

dav-is commented Aug 10, 2018 • edited Loading

TODO

New Configuration Options

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dav-is Aug 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dav-is commented Aug 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dav-is Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

dbo Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dav-is Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dav-is commented Dec 11, 2018

timneutkens commented Dec 11, 2018

Timer commented Dec 11, 2018 • edited Loading

dav-is commented Dec 11, 2018

Timer commented Dec 11, 2018 • edited Loading

dav-is commented Dec 11, 2018

dav-is commented Dec 11, 2018 • edited Loading

Jero786 commented Jan 11, 2019

timneutkens commented Jan 11, 2019

baldurh commented Feb 19, 2019

dav-is commented Feb 19, 2019

timneutkens commented May 29, 2019

monokrome commented Jun 29, 2020

ghost commented Jul 6, 2020

timneutkens commented Jul 6, 2020

javidjamae commented Aug 25, 2020

rogerweb commented Aug 27, 2020

dav-is commented Aug 10, 2018 •

edited

Loading

dav-is Aug 11, 2018 •

edited

Loading

dav-is Aug 28, 2018 •

edited

Loading

dbo Aug 28, 2018 •

edited

Loading

dav-is Aug 28, 2018 •

edited

Loading

Timer commented Dec 11, 2018 •

edited

Loading

Timer commented Dec 11, 2018 •

edited

Loading

dav-is commented Dec 11, 2018 •

edited

Loading