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 HTTP headers #45

Merged
merged 4 commits into from
Sep 26, 2021
Merged

Conversation

sorcix
Copy link
Contributor

@sorcix sorcix commented Sep 24, 2021

Sadly, the Vue web interface needs unsafe-inline for both stylesheets and scripts, making the CSP header a bit useless.

Browsers tend to implement CSP a bit different, so this should be tested in a few browsers. The code changed in this PR is currently live on https://ots.webserve.be and works in Firefox and Chrome.

main.go Outdated Show resolved Hide resolved
Luzifer
Luzifer previously approved these changes Sep 24, 2021
@Luzifer Luzifer dismissed their stale review September 26, 2021 12:53

Please rebase onto master and resolve conflicts

@sorcix
Copy link
Contributor Author

sorcix commented Sep 26, 2021

White testing this I encountered a content encoding error in the view-source option in Firefox.

image

Checking what's causing this.

@sorcix
Copy link
Contributor Author

sorcix commented Sep 26, 2021

It was a missing Content-Type header. Also changed the template a bit.

Should have noticed this while I was checking #47, sorry!

@sorcix
Copy link
Contributor Author

sorcix commented Sep 26, 2021

Should I add Cache-Control headers? I've not added them because I'm not sure if it's a good idea to have this by default, as it could break when people that don't know about them update their instance. I currently add them using an nginx proxy.

main.go Outdated Show resolved Hide resolved
@Luzifer
Copy link
Owner

Luzifer commented Sep 26, 2021

Should I add Cache-Control headers? I've not added them because I'm not sure if it's a good idea to have this by default, as it could break when people that don't know about them update their instance. I currently add them using an nginx proxy.

I think adding Cache-Control header by default will break a lot of things. We're not delivering any parameters for cache invalidation. So depending on the content of the headers this might force users to force-reload to fix broken assets… And from my experience with users they don't know how to do this and will experience a broken application…

Depending on which headers you are adding in the proxy your users might experience this too…

main.go Outdated Show resolved Hide resolved
@sorcix
Copy link
Contributor Author

sorcix commented Sep 26, 2021

I think adding Cache-Control header by default will break a lot of things. We're not delivering any parameters for cache invalidation. So depending on the content of the headers this might force users to force-reload to fix broken assets… And from my experience with users they don't know how to do this and will experience a broken application…

Depending on which headers you are adding in the proxy your users might experience this too…

Agree. I'll keep them in my nginx config.

Luzifer
Luzifer previously approved these changes Sep 26, 2021
main.go Outdated Show resolved Hide resolved
Sadly, the Vue web interface needs unsafe-inline for both stylesheets
and scripts, making the CSP header a bit useless.
Serving the page without Content-Type seems to work but confuses the
view-source mode in Firefox.
Parsing the template for every request makes no sense.
We have only a single variable left, better to remove the map/loop for now.
@Luzifer Luzifer merged commit 14b5801 into Luzifer:master Sep 26, 2021
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

Successfully merging this pull request may close these issues.

2 participants