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

CSP - style-src 'unsafe-inline' should not be required #1050

Closed
acheronfail opened this issue Jan 11, 2019 · 20 comments
Closed

CSP - style-src 'unsafe-inline' should not be required #1050

acheronfail opened this issue Jan 11, 2019 · 20 comments

Comments

@acheronfail
Copy link

acheronfail commented Jan 11, 2019

Expected behavior

react-beautiful-dnd should not depend on the the Content-Security-Policy: style-src 'unsafe-inline' directive.

Actual behavior

react-beautiful-dnd adds errors to the console as the CSS is refused by the CSP rules.

Error messages in console:

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='), or a nonce ('nonce-...') is required to enable inline execution.
Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='), or a nonce ('nonce-...') is required to enable inline execution.
Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-vGSg8WmFAqZHZBYQl6tE4TGs2MeJ6RK2Dhr+EvafjU0='), or a nonce ('nonce-...') is required to enable inline execution.
Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-b117JbKlS2ulpz6LHY5jTjEXzzX7OvRp9yVkQRfn2ag='), or a nonce ('nonce-...') is required to enable inline execution.

The errors originate from here in react-beautiful-dnd.

Possible Solutions:

  1. Add a nonce attribute and make it possible to set the nonce.
    See also: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Unsafe_inline_script.
  2. Use CSS classes rather than unsafe-inline styles.

Steps to reproduce

  1. Use react-beautiful-dnd in a page
  2. Open the page with the Content-Security-Policy: style-src 'self' directive set.

(Alternatively, here is a dead-simple create-react-app repository that you can clone and run to see the errors: https://github.com/acheronfail/react-beautiful-dnd-csp).

What version of react-beautiful-dnd are you running?

10.0.3 (Latest at the time of this issue).

@alexreardon
Copy link
Collaborator

Fascinating!

@alexreardon
Copy link
Collaborator

Would using innerText rather than innerHTML get around the issue?

Alternatively using the Stylesheet api get around this issue?

@acheronfail
Copy link
Author

acheronfail commented Jan 23, 2019

Would using innerText rather than innerHTML get around the issue?

That's a negative.

Alternatively using the Stylesheet api get around this issue?

Also a negative, as quoted here https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/style-src:

Unsafe style expressions

The 'unsafe-eval' source expression controls several style methods that create style declarations from strings. If 'unsafe-eval' isn't specified with the style-src directive, the following methods are blocked and won't have any effect:

CSSStyleSheet.insertRule()
CSSGroupingRule.insertRule()
CSSStyleDeclaration.cssText

Basically, the styles must come from a trusted source, and sources can have different levels of trust depending on the CSP rules defined for the webpage.

To work with almost any CSP configuration, you have the following options (see here for a complete list):

  • Set/configure a nonce attribute on each element/style
    • Although with this approach you must be careful not to allow dynamically generated content into these styles (that would nullify the effects of CSP here)
  • Use CSS classes only and just toggle between classes when needing to set styles
    • This is a win since the content is static and doesn't change, and the user can include/bundle/provide the CSS themselves from a trusted source
  • Provide hashes that can be placed in the CSP header
    • This is a quick fix, but needs to be updated each time the styles change (and doesn't work with dynamically generated styles, etc)

@alexreardon
Copy link
Collaborator

@acheronfail can you confirm if #1037 removes the warnings?

@acheronfail
Copy link
Author

acheronfail commented Jan 23, 2019

Yep! Can confirm that adding a nonce like #1037 does indeed remove the warnings.

Just to confirm though, is there anyway for these styles to contain user/dynamically generated content (that could allow someone to inject unwanted styles, etc)? 🤔

@alexreardon
Copy link
Collaborator

No. The library generates the styles

@m4r71n
Copy link

m4r71n commented Apr 16, 2019

Hi, another solution would be to provide a separate .css file for the preset styles.
Or just an option to disable the preset styles would also help. So we could integrate the preset styles in our own css file.

@alexreardon
Copy link
Collaborator

Interesting. We apply different styles at different points in the lifecycle and change their definition. You can read more about it here:

https://medium.com/@alexandereardon/dragging-react-performance-forward-688b30d40a33

@alexreardon
Copy link
Collaborator

Dynamically updating styles is a solved problem with styled-components, emotion etc. I would be interested to see how they avoid this issue - so we can use that!

@alexreardon
Copy link
Collaborator

Right now we cannot use

Use CSS classes only and just toggle between classes when needing to set styles

As we change the rules on the fly to avoid a re-render to update classes (for performance reasons)

@alexreardon
Copy link
Collaborator

We should also add a test to ensure that there are no CSP errors going forward. Perhaps a puppeteer test would be the most appropriate?

@alexreardon
Copy link
Collaborator

alexreardon commented Apr 23, 2019

I think I would be happy to add the nonce given it gets around the issue relatively easily.

Proposed todo:

@Zweder are you okay to pick this up?

@alexreardon
Copy link
Collaborator

Given that we cannot leverage a class swapping mechanism for performance reasons - lets use this nonce for now.

@Zweder
Copy link

Zweder commented Jun 11, 2019

I have added unit tests to my PR

But, "Write a cypress test with Content-Security-Policy: style-src 'unsafe-inline'" this doesn't make sense, because cypress is the client not the server which should provide the nonce and the csp-header or meta tag and the cypress tests are running against storybook, that makes it very difficult to set headers.

I'd like to make some real-world tests, but i don't like to hack into storybook, they do a fine job!

@alexreardon any thougts?

@alexreardon
Copy link
Collaborator

alexreardon commented Jun 12, 2019

I just want to ensure that we are actually complying with the CSP. I thought a browser test would enable us to be 100% sure. As you mentioned - using storybook complicates it as we don't have control of the server

The effort to spin up a new micro server and stand alone example might not be worth it

@Zweder
Copy link

Zweder commented Jun 12, 2019

Having a look at your browser-test-harness.js spinning up a micro server will not be that difficult and i think i can resure a storybook test without breaking changes.

To keep your 'Unopinionated styling' (which i like) possible in a locked down CSP browser, i'm willing to put in the effort to get the tests in place. Btw emotion and jss are doing the same thing to get this working. I's my spare time so i'll need a couple of days.

@Zweder
Copy link

Zweder commented Jun 15, 2019

I've added a stand alone example and some cypress tests, but there is one thing to mention, cypress strips off the csp header so it doesn't work, but i've added a meta tag in the head with the same policy which also enforces the content-security-policy and that does make it work.

Is it time to profit or am i missing something, comments are welcome

@NikoHelle
Copy link

Hi, just asking status of supporting CSP with 'unsafe-inline' rule. There has been a PR open for 8 months that fixes it, but apparently it wont be merged?

Can't use this awesome library because passing a nonce is not supported.

@Zweder
Copy link

Zweder commented Sep 3, 2019 via email

@alexreardon
Copy link
Collaborator

This has shipped in #1487. See our new csp guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants