Add SameSite to Cookies #28297

cfabianski · 2017-03-05T17:49:38Z

Summary

Add SameSite to the cookies to reinforce the security.
Define Lax as a first step.
Strict is also supported but is too strict as its name suggests 😸

Other Information

SameSite is the new recommended way to keep your website secure. It would make CSRF even more secure.

Some links:

pixeltrix · 2017-03-06T11:32:17Z

@cfabianski is there any risk of breaking an app when upgrading from an older version if we now enable SameSite by default. Also I don't see a test for making sure that it can be disabled.

Generally though 👍

cfabianski · 2017-03-06T12:31:05Z

We could have a global setting to make it backward compatible. Though, I thought Lax would be safe enough to start with.

AndyStabler

looks good, just noticed a teeny tiny typo

...lib/rails/generators/rails/app/templates/config/initializers/cookies_same_site_protection.rb

AndyStabler

🎉

cfabianski · 2017-03-07T14:37:42Z

@pixeltrix does it look better now?

cfabianski · 2017-03-08T20:17:40Z

@maclover7 Do you have any thoughts about it?

maclover7

Coming along very nicely. Two quick things:

Can you also add an entry to Action Pack's CHANGELOG.md about this?
Can you add a test for what happens to the cookies when the config value is :no_protection?

maclover7 · 2017-03-08T22:52:51Z

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -534,6 +543,10 @@ def digest
        def key_generator
          request.key_generator
        end
+
+        def same_site_protection


I'm probably just missing it, but where is this method used? I didn't see any calls in the PR.

It's not but I followed the pattern which is to have access to this as part of the cookie. Want me to remove it?

If you're not calling it anywhere directly, then I'd say to not add the method right now. If it's needed in the future, it's easy to add :)

Here we go :)

cfabianski · 2017-03-08T23:10:34Z

@maclover7 Done

maclover7

👍 looks good to me, but Security Team folks will probably want to review this 😬

cfabianski · 2017-03-09T00:08:02Z

@maclover7 great thanks.
Do I need to contact the security team or will they just review it when available?

maclover7 · 2017-03-09T00:10:57Z

@cfabianski They'll review it when they get the chance

cfabianski · 2017-03-20T19:23:29Z

@maclover7 do you have any update on that one?
I can resolve the conflicts but it would be nice to get an idea on whether or not this is something we might want.

swrobel · 2018-02-07T16:34:54Z

Maybe a good feature for Rails 6?

cfabianski · 2018-02-07T16:57:55Z

@swrobel I hope so! 😺

cfabianski · 2018-02-08T14:37:11Z

@swrobel fixed the conflicts just in case somebody wants to make it part of Rails 6 :) /cc @maclover7

cfabianski · 2018-02-08T15:27:14Z

I'll have a look at the failures and I'll get them fixed in the meantime

gmcgibbon

SameSite is still absent in Rails 6 (2d3a28b) cookie headers. I think it should be added! 👍 If you're still interested in getting this merged, please rebase and squash your commits.

gmcgibbon · 2018-08-11T02:53:20Z

actionpack/CHANGELOG.md

@@ -1,5 +1,15 @@
 ## Rails 6.0.0.alpha (Unreleased) ##

+*   Add SameSite to cookies.
+
+    With this change in place, the CSRF protection will become useless.


You should note that not all browsers support this feature. Also, since this is just text added to a header that browsers magically obey would it really ever deprecate authenticity tokens?

gmcgibbon · 2018-08-11T02:56:01Z

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -364,6 +369,10 @@ def handle_options(options) # :nodoc:

        options[:path] ||= "/"

+        if %i[lax strict].include?(request.cookies_same_site_protection)


I think %i[lax strict] should be moved to a frozen constant.

gmcgibbon · 2018-08-11T02:58:43Z

...lib/rails/generators/rails/app/templates/config/initializers/cookies_same_site_protection.rb

+
+# Specify the SameSite level protection for the cookies
+# Valid options are :no_protection, :lax, and :strict.
+Rails.application.config.action_dispatch.cookies_same_site_protection = :no_protection


Should this be grouped with config/initializers/cookies_serializer.rb as just a cookies.rb initializer?

cfabianski · 2018-08-19T20:06:23Z

Hey @gmcgibbon, thank you for your review. I managed to implement the changes you asked for.
I've also squashed the commits. Let me know if there is anything I can do :)

gmcgibbon

👍 Code and tests look great but I'm still a little hesitant on the changelog entry.

gmcgibbon · 2018-08-20T01:50:40Z

actionpack/CHANGELOG.md

@@ -1,3 +1,13 @@
+*   Add SameSite to cookies.
+
+    With this change in place, the CSRF protection will become useless.


According to the IETF doc, this isn't true:

"SameSite" cookies offer a robust defense against CSRF attack when deployed in strict mode, and when supported by the client. It is, however, prudent to ensure that this designation is not the extent of a site's defense against CSRF, as same-site navigations and submissions can certainly be executed in conjunction with other attack vectors such as cross-site scripting. Developers are strongly encouraged to deploy the usual server-side defenses (CSRF tokens, ensuring that "safe" HTTP methods are idempotent, etc) to mitigate the risk more fully.

You should re-word to describe SameSite as a CSRF protection additive as opposed to a replacement.

I missed the update here sorry. I've updated it as well now (squashed)

gmcgibbon

One last comment regarding the changelog and then this should be good to go! 😄

gmcgibbon · 2018-08-20T15:26:52Z

actionpack/CHANGELOG.md

+    For now, the default is `:no_protection` which makes it backward compatible.
+    By switching from `:no_protection` to `:lax`
+    in `config/initializers/cookies`, the cookies will get
+    more secure than ever.


For this sentence, I would say

By switching from `:no_protection` to `:lax` in `config/initializers/cookies.rb`, cookies won't be sent by browsers in cross-site POST requests. `:strict` will disable cookies from being sent in both cross-site GET and POST requests.

to provide a little more detail on this feature.

cfabianski · 2018-08-20T15:38:00Z

@gmcgibbon Thank you. I've updated it!

gmcgibbon

💯

cfabianski · 2018-08-20T16:07:26Z

@gmcgibbon let's hope we can get it merged now :)

KelseyDH · 2020-01-23T00:54:32Z

@KelseyDH This is similar to what we had to do. We also had to evaluate whether or not to actually mutate the cookie based on the useragent using the logic suggested by the Chromium team as it breaks old clients (as discussed above).

Thanks for sharing that! Fortunately it appears about 80% of clients are on SameSite compatible browsers.

As much as I hate to say it, this seems like something Rails might need to work around. Perhaps this change will get delayed, but my guess is that it will get through eventually...even Rails 6.1 will have to reckon with the choice of breaking some group of browsers or doing some useragent-specific responding.

Yeah Rails will probably need to begin intelligently parsing User-Agent headers for compatibility.

As for the actual cookie editing, we did a manual string edit using regexes.

@HarrisonB I would love to see this sample code if you have it. 🙏 After February 4 there are going to be a lot of Rails developers scrambling to find workarounds.

hibachrach · 2020-01-23T02:53:10Z

@KelseyDH unfortunately I cannot share the code at this time.

lukad · 2020-01-23T08:39:14Z

@KelseyDH I can share our code. We're manipulating the Set-Cookie header string manually as well.

class SameSite
  def initialize(app)
    @app = app
  end

  def call(env)
    status, headers, response = @app.call(env)

    headers['Set-Cookie'] = headers['Set-Cookie'].split("\n").map do |cookie|
      next cookie if cookie.blank?
      next cookie if cookie =~ /;\sSameSite/

      cookie + '; SameSite=None' + '; Secure'
    end.join("\n")

    [status, headers, response]
  end
end

This is a stripped down version. In our production code we also check that we only set SameSite=None for routes specified in the middleware args.

Also note that we're making sure the cookie is Secure because that will be a requirement for this SameSite policy: https://www.chromestatus.com/feature/5633521622188032

p8 · 2020-01-23T09:24:47Z

There's also: https://github.com/pschinis/rails_same_site_cookie
But I haven't used it yet.

sudoremo · 2020-01-23T12:05:22Z

Given this is set to break all Rails applications using Chrome (~80% of browser users) in 14 days... backporting this to Rails 5.x as a patch ASAP should be a high, high priority. Two years ago Google warned about these changes and it's disappointing Rails took this long to add this configuration to the cookies options hash.

@KelseyDH: You got me startled here for a second ;) But if I got the announcement right, Chrome 80 is not disposing all cookies without the SameSite attribute, but just treats them as SameSite=Lax automatically. And, additionally, it won't allow SameSite=None cookies anymore that are not flagged as Secure. So I think all Rails applications that do not rely on Cross-Site cookies should still work without any changes. Is that right?

@kaspth, on an unrelated matter: Sorry for not answering you back then, I was on vacation for a few weeks and was somehow under the impression that it hadn't been merged yet (may need to improve my GitHub skills there^^).

walkersumida · 2020-01-23T14:05:32Z

@p8 It works on Rails 5.

vfonic · 2020-01-27T11:01:38Z

@rafaelfranca

It depends on the definition of warning. I got no email from Google telling us they would unilaterally make a change that would break a lot of things in the internet, even old versions of their own browser if some app, I don't know decide to send SameSite=None by default to every cookie.

I understand your point of view. What everyone here wants is a working internet. The fact that Google acts the way they do, doesn't mean we need to retaliate in the same way. We've been writing fixes for IE for years, not because we loved the browser, but because we wanted our users to be able to use our webapps.

Chrome is the most used browser in the world. It would be great that one of the most popular web frameworks had a full support for it.

wlipa · 2020-02-05T17:52:40Z

The release of this in Chrome has been pushed back a little. "The SameSite-by-default and SameSite=None-requires-Secure behaviors will begin rolling out to Chrome 80 Stable for an initial limited population starting the week of February 17, 2020, excluding the US President’s Day holiday on Monday."

But apps that need cookies to work in a cross-origin manner (like when embedded in an iframe) will need some fix soon.

https://www.chromium.org/updates/same-site

szpasztor · 2020-03-03T09:22:00Z

SameSite enforcements are being rolled out as of Mar 2. Any updates on this ticket? This is starting to cause breaks in some production apps.

Fjan · 2020-03-04T07:57:52Z

It turns out that old Safari browsers are broken with SameSite:None if you rely on cross-site cookies. For the convenience of other people needing to fix this, this monkey patch wil set SameSite:None if it sees a new Chrome browser to get some immediate relieve. Just drop in config/initialisers, should work on Rails 5 & 6, but you do need the latest version of the rack gem. (Edit: added Firefox)

module ActionDispatch
  class Cookies
    class CookieJar #:nodoc:
      def handle_options(options) #:nodoc:

        # Monkey patch to get app to work on Chrome and Firefox work inside iframes again
        if request.ssl? && request.user_agent =~ /Chrome\/8|Firefox/
          options[:same_site] ||= "None";options[:secure] = true
        end

        if options[:expires].respond_to?(:from_now)
          options[:expires] = options[:expires].from_now
        end

        options[:path] ||= "/"

        if options[:domain] == :all || options[:domain] == "all"
          # If there is a provided tld length then we use it otherwise default domain regexp.
          domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP

          # If host is not ip and matches domain regexp.
          # (ip confirms to domain regexp so we explicitly check for ip)
          options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp)
            ".#{$&}"
          end
        elsif options[:domain].is_a? Array
          # If host matches one of the supplied domains without a dot in front of it.
          options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") }
        end
      end
    end
  end
end

yboulkaid · 2020-03-18T11:21:03Z

For anyone running into this, this is already possible today for session cookies:

Rails.application.config.session_store(:cookie_store, same_site: :none)

Note that if you want to use none as an option to same_site, you'll need to be on a rack version that's higher than 2.1.0, see rack/rack@c859bbf.

This is because rails' CookieStore extends Rack::Session::Abstract::Persisted, see

rails/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb

Lines 40 to 42 in f33d52c

    
           # Because CookieStore extends Rack::Session::Abstract::Persisted, many of the 
        
           # options described there can be used to customize the session cookie that 
        
           # is generated. For example:

.

Fjan · 2020-03-18T11:27:43Z

@yboulkaid That will set it on all browsers, so that's fine but only if you do need to worry about old Safari clients.

blackwellsmith · 2020-03-25T21:21:14Z

How do you implement SameSite attribute for cookies in rails? Ok so I have searched and searched. May explanations on what is happening with SameSite. I can't find anything that shows me how or where to do that. It's like I am completely missing something. Can someone share some insight on how to set this attribute in Rails? I need something concise not vague. Thanks!

blackwellsmith · 2020-03-25T21:39:26Z

ok so an update I have SameSite=None but it need to be secure as well any ideas on how todo that?

yeonhoyoon · 2020-03-26T03:20:11Z

@blackwellsmith you can add secure: true option.

blackwellsmith · 2020-03-26T03:33:47Z

I tried that it didn’t work. I am currently trying it with the rails same site cookie gem and I can see the cookie. I am still trouble shooting some issues.

shawndeprey · 2020-08-01T19:59:22Z

@Fjan Solution above worked beautifully as a short term patch given browsers currently sparsely support this config.

It would be nice to have this type of middleware easily configurable in the Rails base config somewhere; for API only applications this new cookie issue is becoming increasingly prevalent.

Fjan · 2020-08-01T20:45:31Z

@shawndeprey Note that the latest Firefox also drops cookies inside iframes unless they are secure and same-site. It's safe to set this on older Firefox browsers, I changed the test in the monkey patch to:
if request.ssl? && request.user_agent =~ /Chrome\/8|Firefox/

shawndeprey · 2020-08-01T21:19:37Z

@Fjan that's good to know and thanks for the updated config. This is super helpful. 👏

earthctzn · 2020-08-28T16:32:30Z

@yboulkaid I tried this on Rails 6.0.3.2 with Rack 2.2.3 and it had no effect on my _session_id cookie. I only got it to work using the https://github.com/pschinis/rails_same_site_cookie that @p8 mentioned above- thanks for that btw! Here is what I tried, what did I miss? Thanks for any responses.

In config/initializers/session_store.rb
Rails.application.config.session_store :cookie_store, key: '_MY_APP', same_site: :none, secure: true
and also tried.
Rails.application.config.session_store( :cookie_store, key: '_MY_APP', same_site: :none, secure: true)

For anyone running into this, this is already possible today for session cookies:
Rails.application.config.session_store(:cookie_store, same_site: :none)
Note that if you want to use none as an option to same_site, you'll need to be on a rack version that's higher than 2.1.0, see rack/rack@c859bbf.

This is because rails' CookieStore extends Rack::Session::Abstract::Persisted, see

rails/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb

Lines 40 to 42 in f33d52c

# Because CookieStore extends Rack::Session::Abstract::Persisted, many of the

# options described there can be used to customize the session cookie that

# is generated. For example:

.

maclover7 added actionpack needs feedback security labels Mar 6, 2017

cfabianski force-pushed the improve-security-with-cookie-same-site branch from 5fa9239 to c950717 Compare March 7, 2017 00:23

AndyStabler suggested changes Mar 7, 2017

View reviewed changes

...lib/rails/generators/rails/app/templates/config/initializers/cookies_same_site_protection.rb Outdated Show resolved Hide resolved

AndyStabler approved these changes Mar 7, 2017

View reviewed changes

maclover7 suggested changes Mar 8, 2017

View reviewed changes

maclover7 approved these changes Mar 9, 2017

View reviewed changes

gmcgibbon reviewed Aug 11, 2018

View reviewed changes

cfabianski force-pushed the improve-security-with-cookie-same-site branch from c740c8c to 4fa8b05 Compare August 19, 2018 19:33

gmcgibbon reviewed Aug 20, 2018

View reviewed changes

cfabianski force-pushed the improve-security-with-cookie-same-site branch from 4fa8b05 to cff096e Compare August 20, 2018 07:07

gmcgibbon reviewed Aug 20, 2018

View reviewed changes

cfabianski force-pushed the improve-security-with-cookie-same-site branch from cff096e to beac956 Compare August 20, 2018 15:37

gmcgibbon approved these changes Aug 20, 2018

View reviewed changes

annaswims mentioned this pull request Jan 27, 2020

Chrome SameSite Cookie settings department-of-veterans-affairs/va.gov-team#5185

Closed

ankane mentioned this pull request Jan 28, 2020

[Idea] Ability to set SameSite and Secure attributes on cookies ankane/ahoy#436

Closed

JasonBarnabe mentioned this pull request Feb 15, 2020

Set secure cookie flags greasyfork-org/greasyfork#652

Closed

epugh mentioned this pull request May 22, 2020

Brower console is logging "Some cookies are misusing the recommended "sameSite" attribute o19s/quepid#130

Closed

sojan-official mentioned this pull request Jul 12, 2020

Bug: Cross site cookie error on chrome chatwoot/chatwoot#919

Closed

ryanfb mentioned this pull request Aug 26, 2020

Cookie “_rails6_session” will be soon rejected because it has the “sameSite” attribute set to “none” or an invalid value, without the “secure” attribute. #39190

Closed

fosterfarrell9 mentioned this pull request Sep 16, 2020

Firefox complains about cookie same site attribute MaMpf-HD/mampf#77

Closed

thatguysimon mentioned this pull request Mar 1, 2021

google_oauth2 Authentication failure! csrf_detected: OmniAuth::Strategies::OAuth2::CallbackError, csrf_detected | CSRF detected omniauth/omniauth-oauth2#58

Closed

stefan-squareweave mentioned this pull request Mar 17, 2021

Set SameSite=lax for session cookie ackama/rails-template#222

Closed

atruskie mentioned this pull request Jul 5, 2021

Changes to support dev work QutEcoacoustics/baw-server#518

Closed

		@@ -364,6 +369,10 @@ def handle_options(options) # :nodoc:

		options[:path] \|\|= "/"

		if %i[lax strict].include?(request.cookies_same_site_protection)

		@@ -1,3 +1,13 @@
		* Add SameSite to cookies.

		With this change in place, the CSRF protection will become useless.

Add SameSite to Cookies #28297

Add SameSite to Cookies #28297

Conversation

cfabianski commented Mar 5, 2017 • edited

Summary

Other Information

pixeltrix commented Mar 6, 2017

cfabianski commented Mar 6, 2017

AndyStabler left a comment

Choose a reason for hiding this comment

AndyStabler left a comment

Choose a reason for hiding this comment

cfabianski commented Mar 7, 2017

cfabianski commented Mar 8, 2017

maclover7 left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfabianski commented Mar 8, 2017

maclover7 left a comment

Choose a reason for hiding this comment

cfabianski commented Mar 9, 2017

maclover7 commented Mar 9, 2017

cfabianski commented Mar 20, 2017

swrobel commented Feb 7, 2018

cfabianski commented Feb 7, 2018

cfabianski commented Feb 8, 2018

cfabianski commented Feb 8, 2018

gmcgibbon left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfabianski commented Aug 19, 2018

gmcgibbon left a comment

Choose a reason for hiding this comment

gmcgibbon Aug 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmcgibbon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfabianski commented Aug 20, 2018

gmcgibbon left a comment

Choose a reason for hiding this comment

cfabianski commented Aug 20, 2018

KelseyDH commented Jan 23, 2020 • edited

hibachrach commented Jan 23, 2020

lukad commented Jan 23, 2020 • edited

p8 commented Jan 23, 2020 • edited

sudoremo commented Jan 23, 2020 • edited

walkersumida commented Jan 23, 2020

vfonic commented Jan 27, 2020

wlipa commented Feb 5, 2020

szpasztor commented Mar 3, 2020

Fjan commented Mar 4, 2020 • edited

yboulkaid commented Mar 18, 2020 • edited

Fjan commented Mar 18, 2020

blackwellsmith commented Mar 25, 2020

blackwellsmith commented Mar 25, 2020

yeonhoyoon commented Mar 26, 2020

blackwellsmith commented Mar 26, 2020

shawndeprey commented Aug 1, 2020

Fjan commented Aug 1, 2020

shawndeprey commented Aug 1, 2020

earthctzn commented Aug 28, 2020 • edited

cfabianski commented Mar 5, 2017 •

edited

maclover7 left a comment •

edited

gmcgibbon left a comment •

edited

gmcgibbon Aug 20, 2018 •

edited

KelseyDH commented Jan 23, 2020 •

edited

lukad commented Jan 23, 2020 •

edited

p8 commented Jan 23, 2020 •

edited

sudoremo commented Jan 23, 2020 •

edited

Fjan commented Mar 4, 2020 •

edited

yboulkaid commented Mar 18, 2020 •

edited

earthctzn commented Aug 28, 2020 •

edited