search

PerlDancer / Dancer

The easiest way to write web applications with Perl (Perl web micro-framework)
http://perldancer.org/
739 stars 211 forks source link

Dancer::Cookie and sameSite attribute #1215

Open isync opened 3 years ago

isync commented 3 years ago

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 commented 3 years ago

This has been be fixed for Dancer2: https://github.com/PerlDancer/Dancer2/issues/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

bigpresh commented 3 years ago

@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 https://github.com/PerlDancer/Dancer/pull/1217

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

Thanks for the nudge also.

joshrabinowitz commented 3 years ago

@bigpresh planning to test this in the next few days

knutov commented 2 years ago

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

I found the problem can be solved this way: https://github.com/PerlDancer/Dancer-Session-Cookie/issues/20