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 Content Security Policy (CSP) #193

Open
ferki opened this issue Apr 20, 2023 · 3 comments
Open

Add Content Security Policy (CSP) #193

ferki opened this issue Apr 20, 2023 · 3 comments

Comments

@ferki
Copy link
Contributor

ferki commented Apr 20, 2023

First, thank you for your and the community's work on Laminar! I'm a happy user, glad to spread the word about it, and even decided to package it for Gentoo a while ago.

A downstream user who prefers to not use GitHub, expressed feedback online about not being able to (easily?) enable their usual Content Security Policy (CSP) headers for Laminar. I offered to act as a proxy for this idea towards this upstream repo.

Based on my own initial research, CSP headers can be set in HTTP response headers, or in a <meta> element of a page. My guess for the appropriate locations in the source code would be src/http.cpp for the former approach, and src/resources/index.html for the latter.

Their proposal was to use a CSP of:

default-src 'self'; script-src 'self' 'unsafe-eval'; style-src 'self' 'unsafe-inline'

I got their permission to "free to do with this as you please", and also preferred ways of attribution for the idea in a potential follow-up commit message.

This would be easy to add as a patch for the Gentoo package, but I feel like it would be generally beneficial for all users, so I would definitely prefer contributing it back properly.

Would you be open to accept such a contribution? In case it is interesting, what would be the preferred way of implementation, and is there anything else to consider or discuss before sending a PR?

@ohwgiles
Copy link
Owner

Thanks for your report and appreciation, always nice to hear.

I'm interested to have this upstreamed if it makes sense, but so far I don't understand the improvement that it offers.

  1. It is almost completely neutralized with unsafe-eval and unsafe-inline
  2. The threat model doesn't make much sense. The only "user" code that Laminar serves is generated by code already running on the server. If this is compromised then you have a bigger problem.
  3. You're encouraged to run Laminar behind a reverse proxy (e.g. to enable HTTPS). Maybe that's a better place to add this header?

@ferki
Copy link
Contributor Author

ferki commented May 4, 2023

  1. It is almost completely neutralized with unsafe-eval and unsafe-inline

That's right, and I believe a part of the story is about improving the situation for users with high security standards or regulations. Perhaps there is a better fitting CSP, and/or the need for unsafe-eval and unsafe-inline could be eliminated.

I don't have much experience with auditing javascript code, but I believe most of the need for more relaxed CSP comes from the dependencies, and not from laminar code itself. Based on quick searches it might be possible to use Vue and Chart.js with a stricter CSP, but I can't judge all the implications, and this might be better explored as a separate issue by more experienced people.

  1. The threat model doesn't make much sense. The only "user" code that Laminar serves is generated by code already running on the server. If this is compromised then you have a bigger problem.

My understanding is that the purpose of CSP is to reduce the attack surface by stating which resources may be loaded and what they are allowed to do. It sounds like setting such expectations explicitly could help avoid unexpected behavior in case of bugs, or make abuse harder in case of an actual attack.

  1. You're encouraged to run Laminar behind a reverse proxy (e.g. to enable HTTPS). Maybe that's a better place to add this header?

That is surely a great place for it, yes 👍 Perhaps it's best to add a note to the documentation about setting CSP on the proxy with the currently required value 🤔 That would help users to understand this aspect better. By raising awareness, someone with a deeper understanding of the involved details may also step up to propose further improvements if they wish so.

Would that be a compatible first step?

@ohwgiles
Copy link
Owner

ohwgiles commented May 7, 2023

Would that be a compatible first step?

Sure. Feel free to open a PR to the user manual and/or the reverse proxy example config.

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

No branches or pull requests

2 participants