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

security: Tighten csp connect-src to prevent external websockets #2705

Merged
merged 3 commits into from
Jun 29, 2022

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jun 28, 2022

This does make the CSP header dynamic (was static). The extra overhead is negligible.

In the future we can drop this and go back to static as Webkit just recently fixed this. Safari's next patch/release will include the webkit fix.

What this does

Currently we allow external websockets to be created. This PR restricts websockets to the host in the request. All non-webkit browsers already did this from the 'self' directive in connect-src.

Notes

Tested with Gnome Web Browser

@Emyrk Emyrk marked this pull request as ready for review June 28, 2022 15:25
@Emyrk Emyrk requested a review from a team as a code owner June 28, 2022 15:25
@Emyrk Emyrk requested a review from kylecarbs June 28, 2022 19:13
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Is there any way to test these reasonably? Seems like something we could easily slip up on that would break the entire app.

site/site.go Outdated Show resolved Hide resolved
site/site.go Outdated Show resolved Hide resolved
site/site.go Outdated Show resolved Hide resolved
site/site.go Outdated Show resolved Hide resolved
site/site.go Show resolved Hide resolved
@Emyrk Emyrk enabled auto-merge (squash) June 29, 2022 15:48
@Emyrk Emyrk merged commit 889e2e6 into main Jun 29, 2022
@Emyrk Emyrk deleted the stevenmasley/csp_connect_src branch June 29, 2022 16:42
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.

None yet

2 participants