trezy / next-safe

https://trezy.gitbook.io/next-safe/
BSD 3-Clause "New" or "Revised" License
176 stars 22 forks source link

`next-safe` as Next.JS 12 Middleware #12

Open UncleClapton opened 3 years ago

UncleClapton commented 3 years ago

still gathering info...

trezy commented 2 years ago

@nibtime has provided a very thorough example implementation for this. https://github.com/vercel/next.js/discussions/31402#discussioncomment-2063781

nibtime commented 2 years ago

Hi,

as I elaborated #36, I found out, that using next-safe as it is, is pretty easy with middleware. This is my current setup:

import _nextSafe from 'next-safe';
import type { Chainable } from 'next-safe-middleware';
import { chain, provideHashesOrNonce } from 'next-safe-middleware';

// because of type bug.
const nextSafe = _nextSafe as unknown as typeof _nextSafe.nextSafe;

const isDev = process.env.NODE_ENV === 'development';

const addSecurityHeaders: Chainable = async (req, res) => {
  const headers = nextSafe({
    isDev,
    contentSecurityPolicy: {
      'script-src': [`'strict-dynamic'`, `'unsafe-inline'`, 'https: http:'],
      'style-src': [`'unsafe-inline'`],
      'connect-src': [
        `'self'`,
        'https://www.datocms-assets.com',
        'https://o1107586.ingest.sentry.io',
        'https://sentry.io',
        'https://vitals.vercel-insights.com',
        'https://site-api.datocms.com',
        ...(req.cookies.__next_preview_data__
          ? ['https://graphql-listen.datocms.com']
          : []),
      ],
      'img-src': [
        `'self'`,
        'data:',
        'https://www.datocms-assets.com',
        'https://i.ytimg.com',
        'https://flagcdn.com',
      ],
    },
  });
  headers.forEach((header) => res.headers.set(header.key, header.value));
};

export const middleware = chain(addSecurityHeaders, provideHashesOrNonce)();

Instead of placing the config in the headers function of next.config.js, you place it in middleware and call headers.forEach((header) => res.headers.set(header.key, header.value));.

It has additional benefits because you can depend the config on request data. For instance, I want https://graphql-listen.datocms.com added to connect-src in Next Preview Mode only (needed for CMS editors, not regular visitors). In middleware, I can express that, in next.config.js I cannot.

Edit: Wrong, you can (see here) but it's really cumbersome I think. In middleware, this is way easier and cleaner with conditional spreading.

This alone I think is simple enough and maybe just a "add to docs" issue?

Then you can do additional stuff further down the chain, like provideHashesOrNonce, which allows you to use 'strict-dynamic' without breaking your app. This requires the additional use of drop-in components in _document. Things like these would be worth packaging up separately, as they would be very complicated to set up yourself and introduce a new class of possible bugs (messing with next/document, prerendering) that have nothing to do with next-safe and CSPs.

kiily commented 2 years ago

Hi @nibtime, would love to be able to help with this. I tried creating the nonce and passing it through to the env via the config but it does not actually update it everytime the page is hit... On your example above, where are the next-safe-middleware utilities coming from, I can't seem to find that in npm. Is this something custom?

nibtime commented 2 years ago

Hi @kiily,

the next-safe-middleware utils are currently a private package within a project monorepo of mine (see #36). It's easier for me to develop there with Vercel Pro. But I am pretty much done now, I got it to work in hybrid mode with all prerendering strategies and fairly simple interfaces. I wanted to get in touch with the maintainer (@trezy) in Discord about integrating this, but I wanted to get this into a good PoC state for all prerendering modes before. There are lots of small quirks and annoyances to handle when messing with next/document and it's not well documented (because it's core framework stuff you are not supposed to touch normally) and I didn't want to waste anybody's time with that. I think I got most of that out of the way now though. When you are interested I'll set it up as a public repo. At this point, it would benefit from being tested and feedback by others I think.

If you want to use it in production I'd strongly recommend something like Sentry (https://docs.sentry.io/product/security-policy-reporting/) and e2e health checks.

I made small ad-hoc demo pages to test and explain stuff to myself:

https://site-zcm-git-nibtime-issue24-zcm.vercel.app/csp-test/dynamic (getServerSideProps)

https://site-zcm-git-nibtime-issue24-zcm.vercel.app/csp-test/static (getStaticProps)

https://site-zcm-git-nibtime-issue24-zcm.vercel.app/csp-test/isr/any-slug-you-like (getStaticProps with fallback and 10s revalidate (ISR))

A bit of background to the problem you encountered and how those next-safe-middleware utils work:

Whether you can use a nonce or not depends on the prerendering strategy of a page. Nonces only work for pages with getServerSideProps, as only there you can refresh them per request. When I first approached this I encountered the exact same problem πŸ˜„

For pages with getStaticProps, it won't refresh because they are build-time prerendered, so nonce doesn't work there, because it's too late to when middleware runs. You can of course generate one in middleware, but you have no chance to set it as an attribute of script tags as prerendering is already done. There you need to use hashes.

To get hashes of routes with getStaticProps, I wrote drop-in components for next/document that write out hashes of each route into the public folder at build-time, so middleware can fetch them and inject them into the CSP header (provideHashesOrNonce)

The chain function is just some sugar to stitch multiple middleware handlers together within the same _middleware file.

kiily commented 2 years ago

Hi @nibtime ,

Thanks for getting back to me and thanks for the very in-depth clarification. I tried an implementation with middleware myself, passing the nonce as a cookie but had some other issues.

I eventually solved the problem for the case of getServerSideProps by using the _document.tsx approach and adding the CSP headers to the response there (which is slightly better than as a meta tag); this would be through the getInitialProps method which runs server-side. Basically, I got these to report to a tool called URIPorts that seemed useful for testing and all of that is checking out. We’re using next-safe for all the basic security but handling the CSP ourselves so we can use the nonce approach

I can put a small repo together to showcase that approach but also keen to be part of the discussion with @trezy. Been looking at making some open-source contributions myself and this seems like an ideal task. Middleware would be a very nice way to fix this. Let me know how I can help with testing or if I you want me to take a look at that code.

nibtime commented 2 years ago

Hi @kiily

you're welcome, I am glad if it was helpful.

I just packaged my solution up and published to NPM: @next-safe/middleware.

I updated my project also to consume the version NPM, so the links above use this package now, before they used a local transpiled version.

If you would test the package in a showcase repo that would be great! I will publish ffrom my private monorepo PR for now. Eventually, the code should become part of an open-source project and properly maintained.

The approach this package solves the case for getServerSideProps is basically the same as yours. But it also solves it for getStaticProps including ISR with hashes and also provides a Trustify component for making it work with custom inline scripts like ga and the such in _document. The document components from the package choose the right strategy for any given page.

The package also includes the source code if you want to have a look at it and compare approaches. I suspect once go into making it work with getStaticProps you will encounter the same problems and the solution would look quite similar.

Using CSP in Meta Tag also has another disadvantage: Report-Only mode doesn't work and can't be integrated with next-safe because it uses headers. So it wasn't an option for creating this package.

kiily commented 2 years ago

Hi @nibtime, that's awesome. I have been quite busy with work but have started testing in a separate repo. I'll update here with what I find once I am done (at some point before the end of this week).

I'll have a look at the implementation you have and compare it with the getServerSideProps that I have. For the others, I think you've spent more than enough time investigating them already πŸ˜† I'll make sure those cases are also tested in my repo though.

Speak soon and thanks again for all the help πŸ™

nibtime commented 2 years ago

Hi @kiily,

I've been in DevOps mode lately for projects of mine, so I attempted a setup for the lib as well: https://github.com/nibtime/next-safe-middleware.

I set it up as a monorepo with yarn 3, swc bundling, releasing with changesets and e2e testing on Vercel. I can at least draft proper releases now, was very annoying from private project PR πŸ˜„. So src is now publicly available and in sync with what's being published to NPM.

I also played around with StackBlitz and made a demo where you can test production builds really quickly: https://stackblitz.com/edit/nextjs-km7zwa

Cached prod rebuilds take around 5-10s to serve. WebContainer seem to be really fast. If you want to test things out and don't have much time I can really recommend that.

I also would advise against going into the getStaticProps and especially the ISR case, it was very very annoying sometimes, strange things can happenπŸ˜„.

kiily commented 2 years ago

Damn @nibtime, you've been absolutely smashing it, that's amazing! I'll give it a go on the StackBlitz, things were working well on my example repo. Going to tidy it up and publish it here as well. Thank you so much for your time and help πŸ™

nibtime commented 2 years ago

Thanks @kiily πŸ˜„. I am glad that was helpful for you.

I've just released 0.4.0 and also updated the StackBlitz with the new version/demo features. This version works around some weird SRI (subresource integrity) bug of Firefox killing Hash-based strict CSP and also delivers a middleware to set up reporting. I started using this version on a new project in production in report-only mode and funnel the violation reports into Sentry, so I'll get some insight on how all that plays out for real.

I've never done/published any public project before, but so far it's been a really fun way for me to learn and experiment with great new tech. If you like to participate at some time you're more than welcome.

kiily commented 2 years ago

@nibtime terribly sorry for such a long silence... A lot of work got in the way and I am now looking at this again πŸ™ I'm stuck on a peerDependency issue for now but hopefully can take a proper look at it once it is resolved - see here https://github.com/trezy/next-safe/issues/39.

Any tools you can recommend for testing that the CSP is all correct before going live would be great. I'll make sure to report back this time if I finally get it all working πŸ™