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 Rack::Protection::ReferrerPolicy #1291

Merged
merged 1 commit into from Mar 13, 2020
Merged

Add Rack::Protection::ReferrerPolicy #1291

merged 1 commit into from Mar 13, 2020

Conversation

stefansundin
Copy link
Contributor

Hi everyone. I decided to implement the Referrer-Policy header for rack-protection. It's a really simple header with just a string value, more information:

I considered making it enabled by default, since it has low risk of breaking the web, but I want your opinion first.

Worth noting is that the default value I picked, "strict-origin-when-cross-origin", does not work in Chrome at the moment. I picked it as it will be the most sensible default in the future, especially if this is enabled by default. See this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=627968

@Ice-Storm
Copy link
Contributor

many browers not support this header. but it's future ~

@olleolleolle
Copy link
Member

@stefansundin Thanks for creating this.

The Chrome issue you linked to has since been marked fixed.

Implement new referrer policies

This CL implements the policies 'same-origin' and 'strict-origin', and
repurposes existing logic that was previously only available behind a flag for
'strict-origin-when-cross-origin'. (I've left it as a TODO for a follow-up
to rename this policy to match the spec.)

Existing web platform tests cover the new policies and should now pass.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/TgtPUowSWuU/Y-Sn2oRsCAAJ

Does that fact change any of your earlier assessment of the mergeability of this change?

@stefansundin
Copy link
Contributor Author

That note in the issue is confusing to say the least. Anyway, when Chrome 61 ships, it will be easy to test if the default value works as expected.

@zzak
Copy link
Member

zzak commented Aug 20, 2017

Let's wait for chrome 61 to test this then. My reading of this seems to be that this name is only temporary? And will likely change to match the spec? Not sure which spec they're... referring to here tho :)

@stefansundin
Copy link
Contributor Author

This post suggests that the policy name hasn't changed: https://blog.chromium.org/2017/08/chrome-61-beta-javascript-modules.html

I think the note was talking about renaming something internally. But we can still wait.

@namusyaka namusyaka added this to the Beyond milestone Feb 19, 2018
@stefansundin
Copy link
Contributor Author

@zzak I think this can merged now. :)

@jkowens jkowens merged commit fade5fe into sinatra:master Mar 13, 2020
@jkowens
Copy link
Member

jkowens commented Mar 13, 2020

Thanks @stefansundin, I think at some point we may want to circle back and enable this protection by default.

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

6 participants