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

Dancer::Cookie and sameSite attribute #1215

Open
isync opened this issue Jun 13, 2020 · 8 comments
Open

Dancer::Cookie and sameSite attribute #1215

isync opened this issue Jun 13, 2020 · 8 comments

Comments

@isync
Copy link
Contributor

isync commented Jun 13, 2020

Firefox started to complain in developer console that:

Cookie “some_session” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. To know more about the “sameSite“ attribute, read https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite

Okay? That's new. Some reading later I found that it's a new XSS prevention scheme championed by Google. Browsers will start to look for an additional string in Cookies, an attribute... Well, should I worry about it now? Mh. But when FF starts to complain about it, .. it will probably be an issue in a few months time.

That's why I dug around in Dancer module sources. I use Dancer::Session::Cookie a lot and first thought the root of that warning would be there. Wrong. As it seems for me, it's deeper in Dancer's core, namely in Dancer::Cookie.pm Please tell me if I'm wrong: from what I see there's no means in _to_header() to set an additional attribute to the raw cookie string generated there. Even if calling code, modules like Dancer::Session::Cookie, would care about it, try to set it, etc.

Am I on the wrong track here? Or, if not: did anyone else run into this? Or is there already a stance of the Dancer devs on it, why it's not cared about?

Thanks and cheers to everyone on the Dancer team for making one of the finest Module/frameworks on CPAN!

@racke
Copy link
Member

racke commented Jun 14, 2020

This has been be fixed for Dancer2: PerlDancer/Dancer2#1547. So we ran into it, but Dancer2 is recommended and Dancer is only around for existing projects. It would be good to fix that in Dancer as well.

@bigpresh
Copy link
Member

As soon as I get a moment I'll try to fix this in Dancer v1. I concur with racke that Dancer2 is the right choice for new projects, but I plan to keep Dancer 1 supposed for important bug / security fixes for as long as I can (although some of my reason to do so is gone, now that I have changed employers; my previous employer relied on Dancer 1 heavily, so would effectively sponsor some of my time working on D1, and that is no longer the case.)

@bigpresh
Copy link
Member

I've started the work on supporting the SameSite attribute, just need to finish the tests for it and make sure it works correctly.

In the meantime, I'm wondering whether Dancer should send "None" by default if the user hasn't specified a setting, or just not set the attribute at all unless the user has specified it. At a quick glance I think Dancer2 does the latter, so that's probably the model to follow, unless anyone has any good arguments otherwise?

From my reading of the stuff about the SameSite attribute, it only affects third-party requests (e.g. a third-party site on another domain pulling stuff from your Dancer app), so for a lot of people, there would be little impact from this change.

@isync
Copy link
Contributor Author

isync commented Jun 22, 2020

From my reading of the "sameSite stuff", I see it as a "yet another new string that piles up in the slew of semicolon-separated optional values in a cookie string". And although all modern browsers would probably handle parsing these right, I would second the approach to not not set anything unless it's stated by the user in Dancer's config / or Dancer code, asking it to be set. In other words: no "none" or anything should end up in a cookie string, but when the user specifies a rule for handling of sameSite, it will be set/appended. Just my 2c... High five, presh, for actually investing time and effort!! Thank you.

@joshrabinowitz
Copy link
Contributor

@bigpresh can I do anything to help port this over from dancer2 and test?

@bigpresh
Copy link
Member

@bigpresh can I do anything to help port this over from dancer2 and test?

I've chucked my work on it as a draft PR at #1217

Please feel free to review & test if you have a moment at all!

Thanks for the nudge also.

@joshrabinowitz
Copy link
Contributor

@bigpresh planning to test this in the next few days

@knutov
Copy link

knutov commented Jan 4, 2022

Original question seems to be about Dancer::Session::Cookie, not Dancer::Cookie.

I found the problem can be solved this way: PerlDancer/Dancer-Session-Cookie#20

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

5 participants