Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add security headers by default (with ability to override or disable) #23993

Closed
leerob opened this issue Apr 12, 2021 · 14 comments
Closed

Add security headers by default (with ability to override or disable) #23993

leerob opened this issue Apr 12, 2021 · 14 comments
Labels
Developer Experience Issues related to Next.js logs, Error overlay, etc. linear: next Confirmed issue that is tracked by the Next.js team.
Milestone

Comments

@leerob
Copy link
Member

leerob commented Apr 12, 2021

Describe the feature you'd like to request

Next.js provides the ability to set custom headers, but you need to do the research to understand which security headers should be included to secure your site. It would be great if Next.js had sensible defaults that users could override or disable if not needed. This would provide more security out of the box.

Describe the solution you'd like

By default, we should we some (or all) of the security headers listed in this tweet: https://twitter.com/leeerob/status/1381605537742254082.

We should also generate a least privilege CSP. This will require adding documentation as well, so folks understand why these have been added, what they do, and how to turn them off.

Describe alternatives you've considered

Manually adding security headers in next.config.js. Currently, we have added documentation with some recommendation for headers to add: https://nextjs.org/docs/advanced-features/security-headers

Resources

NEXT-1376

@adamdottv
Copy link

adamdottv commented Apr 13, 2021

I mentioned in reply to the referenced tweet that Ness (https://github.com/nessjs/ness) implements a similar feature when deploying sites to AWS. For the security headers, it typically uses an origin response handler, but when deploying Next.js sites it has to use a (less ideal) viewer response handler, simply because Next.js already needs to leverage the origin response slot.

For the CSP generation, I wrote a semi-naive implementation using cheerio to parse the build output HTML files. Obviously, this can fall over in dynamic content scenarios, but anecdotally it's worked great for me on a dozen or so Next.js sites. I think it'd be good to get someone with more of a web security background to review, but the CSPs this code generates do pass the observatory.mozilla.org test (A+).

If bringing this functionality into Next.js is attractive and I can be helpful in any way, I'm here for it.

@leerob
Copy link
Member Author

leerob commented Jun 21, 2021

In the meantime, I've written some documentation: https://nextjs.org/docs/advanced-features/security-headers.

@trezy
Copy link

trezy commented Jun 25, 2021

I definitely think we should be helping developers by setting stricter default security on Next.js, but setting default CSP headers could be a tough pill for new devs to swallow if they're not already familiar with it. Here are a few scenarios to consider:

Scenario 1: All content is static and controlled by the site owner

Example: Software documentation site generated from Markdown files.

This is a pretty easy one. We could go for a system like @adamelmore built for ness and extract URLs from static source, and everything would be gravy.


Scenario 2: All content is static, but includes user-generated content

Example: A developer blog that allows users to use images in comments on posts.

This one gets a lot more tricky. If we extract URLs from the static source and use those to generate a CSP, we're giving the developer a false sense of security. If a static page is generated with user comments, they may include URLs to malicious (or just undesirable) URLs that should really be blocked by the CSP. If we go this route, we should definitely provide some kind of warning to the developer that they should enforce their own CSP rather than relying on what's provided by Next.


Scenario 3: Some scripts are injected dynamically

Example: Any site that uses a tag manager (like Google Tag Manager or Adobe Launch) that injects other scripts after load.

This is the toughest one of all. If we go the route of pulling URLs from static source, we'll only catch the tag manager URL. We won't capture any of the other URLs that would be injected after load on the client. This results in the developer's tag manager being broken without a clear reason why.


Let me be clear: none of this is to discourage Next from implementing a default CSP. Rather, I want to point out some important issues that I've considered and encountered while working on next-safe.

At the moment, I'm leaning more towards enabling the user to write a good CSP, rather than setting defaults that could become onboarding hurdles for developers new to Next. Admittedly, I'm biased there because I think everybody should just use my library. 😛

That said, one of the biggest downfalls of next-safe is that it can't get a nonce when creating the CSP headers because the headers() function in next.config.js doesn't receive any arguments. Instead, we have to use unsafe-inline to avoid breaking Next all together. We can actually achieve a stronger CSP via a <meta> tag added to <NextHead>, but that has its own downsides (like MITM attacks that would inject scripts before the <meta> tag).

I've been meaning to dig into Next and see if there's a good way to set a nonce on all of the script tags, then pass that nonce to the headers() function. This would allow us to generate strong headers with strict CSP, rather than having to use unsafe-inline because of the inline scripts that are required. Even better would be generating hashes for each of the inline scripts used and passing those to the headers() function, but I'd be just as happy with a nonce for now. 🤷🏻‍♂️

@Vadorequest
Copy link
Contributor

Well, from above comments it looks like people writing libs are into this security stuff and aim at making it easier and more manageable for other people. That's great!

Actually, it's @leerob who got me there in the first place by his tweet 😂

As I dived deeper into this, I bookmarked a few useful links:

And eventually tried to improve Next Right Now default headers configuration, not only focusing on security but also on performances. (caching static assets and fonts, mostly)

I also strongly believe it's the role of the Next.js framework to push developer to do better. And it mostly starts by educating us. (which Lee does a very good job at, IMHO)

https://nextjs.org/docs/advanced-features/security-headers is a good start. 🎉
I think there should be some warnings when starting the Next.js server locally, too. (that's what I'd look at first, way before diving into some hidden documentation gem)

What's most complicated is to find a common ground for everyone, every app has its own rules, some need to be embedded into other websites, others don't. I also tried to make it opt-in with a flag to make it easy to customise for people using NRN. I eventually decided I wouldn't dive too deep into it, to avoid over-complicating things.

The CSP -in particular- looks horrible to write/maintain/debug, when I looked at the specs I was like "I don't want to do that, it's way too complicated, too much ground to cover". I thought Next.js could help me with that (and that's what https://github.com/trezy/next-safe tries to do).

I'd love for the framework to educate us more on those topics, we don't know what we don't know, and it's impossible to know the best practices of every tech-related things out there.

@trezy
Copy link

trezy commented Jun 25, 2021

I'd love for the framework to educate us more on those topics, we don't know what we don't know, and it's impossible to know the best practices of every tech-related things out there.

@Vadorequest nailed it. It would be great to see Next.js replace next-safe with something native and opt-in, and I think the messaging and education around that is the most important piece. 😊

@adamdottv
Copy link

Even better would be generating hashes for each of the inline scripts

I'm fairly certain this is what I'm doing with Ness, but I have the benefit of adding the headers in a CloudFront function on viewer-response, since Ness manages deploys.

@Vadorequest
Copy link
Contributor

[...] and I think the messaging and education around that is the most important piece.

Exactly. Even if Next.js can't optimize everything automatically for us, it should at least warn/educate us about the important stuff. There are a few things that can be automated, but I feel like most of the work needs to be done by the developers themselves.

I'm not only focusing on security here, but best-practices in general, not matter if they're related to performances, security, UX, etc.

For instance, I took me two years to figure out I wasn't caching any asset (images, scripts, fonts). I somehow assumed it was built-in, and I never looked into it by myself. (and yet again, it's one's of Lee's tweets that got me thinking about it!)

But, have I had a warning in the server console at the start of the app stating something like "No Cache-Control value defined in next.config.js, are you sure you don't have any static asset that could benefit from caching? [link]" would have educated me about it, and I'd have improved my knowledge of Cache-Control header and how/when to use it.

It's a very simple example, but I bet it's very common. I know Next.js has telemetry usage, maybe they have stats such as what % of projects use a Cache-Control header. I assume 99.5+% should use at least one Cache-Control directive.

@mikesherov
Copy link

Hi, just found this thread, and looking forward to ease-of-use improvements for CSP headers. Is there any reason to suggest a CSP header other than a nonce-based policy with strict-dynamic and unsafe-inline fallback? If next.js made it easy to share a nonce between header config and script and style tag generation, wouldn't a "catch-all" "good" default policy (for script and style) be:

For pages that are not SSG:

`Content-Security-Policy: script-src 'nonce-${nonce}' 'unsafe-inline' 'strict-dynamic';`

For SSG pages:

`Content-Security-Policy: script-src 'sha-${shaOfScript1}' 'sha-${shaOfScript2}' 'unsafe-inline' 'strict-dynamic';`

I'm trying to understand if there are caveats to these approaches, and if not, is there anything other than time and effort preventing this from being implemented as such?

@trezy
Copy link

trezy commented Jun 29, 2021

Is there any reason to suggest a CSP header other than a nonce-based policy with strict-dynamic and unsafe-inline fallback?

Yeah, the biggest issue here is that Safari doesn't yet support strict-dynamic, which would cause all sorts of headaches for sites with tag managers (or other client-side script loaders). Once Safari supports strict-dynamic, though, I think that's a pretty solid default.

@mikesherov
Copy link

Sorry, I should have been clear. Since strict-dynamic will nullify self, unsafe-eval and source lists, wouldn't this be a policy that is a safe default for non-SSG:

script-src 'nonce-r@nd0m' 'strict-dynamic' 'unsafe-inline' https: 'self'

As per: https://content-security-policy.com/strict-dynamic/

@shmdhussain
Copy link

How can we have different nonce value for static pages (may contain user infos loaded through apis if user logged in).

with next js, all the pages are cached either by cdn or nextjs revalidate time.

My current thinking is Do we have to generate the page for user , every time when user requests the page , with different nonce value

Is there any better solutions there?.

Strict-dynamic nonce-key is the way to go for me to implement google tag manager(bcz I need trust cascade for scripts loaded by google tag manager), but with caching this makes nonce value ineffective.

@mrbeardad
Copy link

Has there been any progress on this issue?

@timneutkens timneutkens added the linear: next Confirmed issue that is tracked by the Next.js team. label Jul 4, 2023
@pfista
Copy link

pfista commented Aug 25, 2023

Also wondering how we can set script-src to strict-dynamic using either nonces or hashes with Next.js/Vercel.

@leerob
Copy link
Member Author

leerob commented Sep 1, 2023

Hey folks, wanted to swing back here with an update. After digging through many different issues and discussions, I've made a new page in the documentation (PR) specifically for Content Security Policy and nonces. This docs page:

  • Explains how to generate a nonce with Middleware
  • Shows how to consume the nonce in a route with headers()
  • Shows a complete CSP without needing to use any unsafe
  • Shows how to ignore the nonce Middleware from running on prefetches / static assets

Further, we've patched some bugs and made improvements to nonce handling in Next.js that will be available in the latest canary version (for those of you time traveling from the future, upgrade to Next.js 13.5). We also updated the with-strict-csp example in the examples/ folder, which is backlinked from the new documentation page.

Really hope this helps out, thank you all 🙏 I'll be closing this issue out. To continue the discussion, please go here. I feel this is the best solution going forward to allow folks to easily add CSP / nonces without adding additional configuration into the framework.

@leerob leerob closed this as completed Sep 1, 2023
@vercel vercel locked as resolved and limited conversation to collaborators Sep 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Developer Experience Issues related to Next.js logs, Error overlay, etc. linear: next Confirmed issue that is tracked by the Next.js team.
Projects
None yet
Development

No branches or pull requests

10 participants