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.

Sign up for GitHub

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

Jump to bottom

Allow CSP to fallback to default-src #1490

Merged
merged 3 commits into from Mar 13, 2020
Merged

Allow CSP to fallback to default-src #1490

merged 3 commits into from Mar 13, 2020

Conversation

jkowens
Copy link
Member

@jkowens jkowens commented Nov 4, 2018

Resolves #1484.

@jkowens
Allow content source to fallback to default-src
4758d5f 
Remove defaults for script-src, style-src, connect-src, and img-src
so that they can fallback to default-src. The default for default-src
has been changed from 'none' to 'self'. This seems to be a safe default
especially as browsers implement prefetch-src. If stricter policies are
needed they can be specified when loading this middleware.
@jkowens
Add support for webrtc-src, navigate-to, and prefetch-src directives
c4485fb
@namusyaka namusyaka self-requested a review December 8, 2018 14:49
@namusyaka
Copy link
Member

This seems good but this has a possibility of making developers be surprised a bit because the default-src is set to self so the developer's site will disable inline-javascript and iframes.
What do you think?

@jkowens
Copy link
Member Author

jkowens commented Dec 9, 2018 •
edited

script-src is currently being set to self so I don't think that will cause any surprises. For frame-src and some other less common options, they will fallback to 'self' instead of 'none' now, so that is a change. I think a default-src of 'self' provides a measure of security by default, but allows developers to implement stricter (or less strict) policies. If we set default-src to 'none', I think there will be more surprises when developers find scripts are being allowed that they didn't expect.

@namusyaka
Copy link
Member

Yes, script-src, img-src, style-src and connect-src aren't changed by this patch, but other many resources will be changed from none to self by default.
I think original code is not valid but your change breaks compatibility around default values.

To avoid breaking compatibility, we need to write tricky hack unfortunately.
I think we should release this at next minor or major versions (2.x, 3.0), thoughts?

@jkowens
Copy link
Member Author

jkowens commented Dec 10, 2018

Yes, this would probably be best reserved for a major release. My only concern is that releases are few and far between these days since Sinatra development has slowed down. I think it might be necessary to cut new releases with smaller groups of changes for the sake of those that are waiting for fixes.

@namusyaka
Copy link
Member

True. We need more active core members.
I would also like to define the release cycle explicitly.
I'm going to create the prototype idea soon.

@namusyaka namusyaka added this to the v2.1.0 milestone Dec 15, 2018
@jkowens jkowens merged commit c2705ce into sinatra:master Mar 13, 2020
@jkowens jkowens mentioned this pull request Mar 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@namusyaka namusyaka Awaiting requested review from namusyaka

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.1.0
Development

Successfully merging this pull request may close these issues.

Remove Rack::Protection::ContentSecurityPolicy defaults
2 participants
@jkowens @namusyaka