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

Create _document for Content-Security-Policy and other headers #133

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

vingkan
Copy link
Contributor

@vingkan vingkan commented Jul 28, 2021

NextJS has some struggles related to implementing a Content-Security-Policy.

Long story short, in development NextJS generates and evaluates scripts and styles in an unsafe way. It required some messing around with how our NextJS pages are rendered to add a more secure Content-Security-Policy and keep the current functionality.

For more background, check out these links:

The advanced feature in the NextJS documentation did not work. But I was able to use the _document.js page and most of Rees Morris' solution to add the headers. The main change I made, so that the headers would work on our static site in production, was to manually set a meta tag in the Head tag with the header.

Since our site is statically rendered, the nonce will be the same value for each build and available in the source code. I worry that this means attackers can retrieve the nonce and use it to inject a script past the policy. However, I tried to inject a script from a different origin with the same nonce, and it was still rejected by the policy.

I will work with @Jmacias2019 to implement our Content-Security-Policy. Hopefully we can use this approach for the other headers as well.

CC: @olakukielko

@vingkan vingkan added app Work for frontend app enhancement New feature or request security Cybersecurity for the system. labels Jul 28, 2021
@vingkan vingkan requested a review from Jmacias2019 July 28, 2021 15:44
Copy link
Collaborator

@Jmacias2019 Jmacias2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vingkan vingkan merged commit 5c6763b into main Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Work for frontend app enhancement New feature or request security Cybersecurity for the system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants