Skip to content

Commit

Permalink
Add SameSite protection to every written cookie
Browse files Browse the repository at this point in the history
Enabling `SameSite` cookie protection is an addition to CSRF protection,
where cookies won't be sent by browsers in cross-site POST requests when set to `:lax`.

`:strict` disables cookies being sent in cross-site GET or POST requests.

Passing `:none` disables this protection and is the same as previous versions albeit a `; SameSite=None` is appended to the cookie.

See upgrade instructions in config/initializers/new_framework_defaults_6_1.rb.

More info [here](https://tools.ietf.org/html/draft-west-first-party-cookies-07)

_NB: Technically already possible as Rack supports SameSite protection, this is to ensure it's applied to all cookies_
  • Loading branch information
cfabianski authored and kaspth committed Dec 15, 2019
1 parent 4500ebb commit 7ccaa12
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 35 deletions.
17 changes: 17 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
* Add SameSite protection to every written cookie.

Enabling `SameSite` cookie protection is an addition to CSRF protection,
where cookies won't be sent by browsers in cross-site POST requests when set to `:lax`.

`:strict` disables cookies being sent in cross-site GET or POST requests.

Passing `:none` disables this protection and is the same as previous versions albeit a `; SameSite=None` is appended to the cookie.

See upgrade instructions in config/initializers/new_framework_defaults_6_1.rb.

More info [here](https://tools.ietf.org/html/draft-west-first-party-cookies-07)

_NB: Technically already possible as Rack supports SameSite protection, this is to ensure it's applied to all cookies_

*Cédric Fabianski*

* Bring back the feature that allows loading external route files from the router.

This feature existed back in 2012 but got reverted with the incentive that
Expand Down
9 changes: 8 additions & 1 deletion actionpack/lib/action_dispatch/middleware/cookies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ def cookies_serializer
get_header Cookies::COOKIES_SERIALIZER
end

def cookies_same_site_protection
get_header Cookies::COOKIES_SAME_SITE_PROTECTION
end

def cookies_digest
get_header Cookies::COOKIES_DIGEST
end
Expand Down Expand Up @@ -181,6 +185,7 @@ class Cookies
COOKIES_SERIALIZER = "action_dispatch.cookies_serializer"
COOKIES_DIGEST = "action_dispatch.cookies_digest"
COOKIES_ROTATIONS = "action_dispatch.cookies_rotations"
COOKIES_SAME_SITE_PROTECTION = "action_dispatch.cookies_same_site_protection"
USE_COOKIES_WITH_METADATA = "action_dispatch.use_cookies_with_metadata"

# Cookies can typically store 4096 bytes.
Expand Down Expand Up @@ -431,7 +436,9 @@ def handle_options(options)
options[:expires] = options[:expires].from_now
end

options[:path] ||= "/"
options[:path] ||= "/"
options[:same_site] ||= request.cookies_same_site_protection
options[:same_site] = false if options[:same_site] == :none # TODO: Remove when rack 2.1.0 is out.

if options[:domain] == :all || options[:domain] == "all"
# If there is a provided tld length then we use it otherwise default domain regexp.
Expand Down
90 changes: 58 additions & 32 deletions actionpack/test/dispatch/cookies_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,38 @@ def setup
@request.env["action_dispatch.encrypted_signed_cookie_salt"] = ENCRYPTED_SIGNED_COOKIE_SALT
@request.env["action_dispatch.authenticated_encrypted_cookie_salt"] = AUTHENTICATED_ENCRYPTED_COOKIE_SALT

@request.env["action_dispatch.cookies_same_site_protection"] = :lax
@request.host = "www.nextangle.com"
end

def test_setting_cookie_with_no_protection
@request.env["action_dispatch.cookies_same_site_protection"] = :none

get :authenticate
assert_cookie_header "user_name=david; path=/" # TODO: append "; SameSite=None" when rack 2.1.0 is out and bump rack dependency version.
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie_with_misspelled_protection_raises
@request.env["action_dispatch.cookies_same_site_protection"] = :funky

error = assert_raise ArgumentError do
get :authenticate
end
assert_match "Invalid SameSite value: :funky", error.message
end

def test_setting_cookie_with_strict
@request.env["action_dispatch.cookies_same_site_protection"] = :strict

get :authenticate
assert_cookie_header "user_name=david; path=/; SameSite=Strict"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie
get :authenticate
assert_cookie_header "user_name=david; path=/"
assert_cookie_header "user_name=david; path=/; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

Expand All @@ -381,39 +407,39 @@ def test_setting_the_same_value_to_permanent_cookie

def test_setting_with_escapable_characters
get :set_with_with_escapable_characters
assert_cookie_header "that+%26+guy=foo+%26+bar+%3D%3E+baz; path=/"
assert_cookie_header "that+%26+guy=foo+%26+bar+%3D%3E+baz; path=/; SameSite=Lax"
assert_equal({ "that & guy" => "foo & bar => baz" }, @response.cookies)
end

def test_setting_cookie_for_fourteen_days
get :authenticate_for_fourteen_days
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000"
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie_for_fourteen_days_with_symbols
get :authenticate_for_fourteen_days_with_symbols
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000"
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie_with_http_only
get :authenticate_with_http_only
assert_cookie_header "user_name=david; path=/; HttpOnly"
assert_cookie_header "user_name=david; path=/; HttpOnly; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie_with_secure
@request.env["HTTPS"] = "on"
get :authenticate_with_secure
assert_cookie_header "user_name=david; path=/; secure"
assert_cookie_header "user_name=david; path=/; secure; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

def test_setting_cookie_with_secure_when_always_write_cookie_is_true
old_cookie, @request.cookie_jar.always_write_cookie = @request.cookie_jar.always_write_cookie, true
get :authenticate_with_secure
assert_cookie_header "user_name=david; path=/; secure"
assert_cookie_header "user_name=david; path=/; secure; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
ensure
@request.cookie_jar.always_write_cookie = old_cookie
Expand All @@ -428,7 +454,7 @@ def test_not_setting_cookie_with_secure
def test_multiple_cookies
get :set_multiple_cookies
assert_equal 2, @response.cookies.size
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000\nlogin=XJ-122; path=/"
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000; SameSite=Lax\nlogin=XJ-122; path=/; SameSite=Lax"
assert_equal({ "login" => "XJ-122", "user_name" => "david" }, @response.cookies)
end

Expand All @@ -439,14 +465,14 @@ def test_setting_test_cookie
def test_expiring_cookie
request.cookies[:user_name] = "Joe"
get :logout
assert_cookie_header "user_name=; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
assert_equal({ "user_name" => nil }, @response.cookies)
end

def test_delete_cookie_with_path
request.cookies[:user_name] = "Joe"
get :delete_cookie_with_path
assert_cookie_header "user_name=; path=/beaten; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; path=/beaten; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
end

def test_delete_unexisting_cookie
Expand Down Expand Up @@ -723,7 +749,7 @@ def test_permanent_signed_cookie
def test_delete_and_set_cookie
request.cookies[:user_name] = "Joe"
get :delete_and_set_cookie
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000"
assert_cookie_header "user_name=david; path=/; expires=Mon, 10 Oct 2005 05:00:00 -0000; SameSite=Lax"
assert_equal({ "user_name" => "david" }, @response.cookies)
end

Expand Down Expand Up @@ -909,134 +935,134 @@ def test_cookie_with_hash_value_not_modified_by_rotation
def test_cookie_with_all_domain_option
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_a_non_standard_tld
@request.host = "two.subdomains.nextangle.local"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_australian_style_tld
@request.host = "nextangle.com.au"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com.au; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com.au; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_uk_style_tld
@request.host = "nextangle.co.uk"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.co.uk; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.co.uk; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_host_with_port
@request.host = "nextangle.local:3000"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_localhost
@request.host = "localhost"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; path=/"
assert_cookie_header "user_name=rizwanreza; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_ipv4_address
@request.host = "192.168.1.1"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; path=/"
assert_cookie_header "user_name=rizwanreza; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_ipv6_address
@request.host = "2001:0db8:85a3:0000:0000:8a2e:0370:7334"
get :set_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=rizwanreza; path=/"
assert_cookie_header "user_name=rizwanreza; path=/; SameSite=Lax"
end

def test_deleting_cookie_with_all_domain_option
request.cookies[:user_name] = "Joe"
get :delete_cookie_with_domain
assert_response :success
assert_cookie_header "user_name=; domain=.nextangle.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; domain=.nextangle.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
end

def test_cookie_with_all_domain_option_and_tld_length
get :set_cookie_with_domain_and_tld
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.com; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_a_non_standard_tld_and_tld_length
@request.host = "two.subdomains.nextangle.local"
get :set_cookie_with_domain_and_tld
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_a_non_standard_2_letter_tld
@request.host = "admin.lvh.me"
get :set_cookie_with_domain_and_tld
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.lvh.me; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.lvh.me; path=/; SameSite=Lax"
end

def test_cookie_with_all_domain_option_using_host_with_port_and_tld_length
@request.host = "nextangle.local:3000"
get :set_cookie_with_domain_and_tld
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.nextangle.local; path=/; SameSite=Lax"
end

def test_deleting_cookie_with_all_domain_option_and_tld_length
request.cookies[:user_name] = "Joe"
get :delete_cookie_with_domain_and_tld
assert_response :success
assert_cookie_header "user_name=; domain=.nextangle.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; domain=.nextangle.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
end

def test_cookie_with_several_preset_domains_using_one_of_these_domains
@request.host = "example1.com"
get :set_cookie_with_domains
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=example1.com; path=/"
assert_cookie_header "user_name=rizwanreza; domain=example1.com; path=/; SameSite=Lax"
end

def test_cookie_with_several_preset_domains_using_other_domain
@request.host = "other-domain.com"
get :set_cookie_with_domains
assert_response :success
assert_cookie_header "user_name=rizwanreza; path=/"
assert_cookie_header "user_name=rizwanreza; path=/; SameSite=Lax"
end

def test_cookie_with_several_preset_domains_using_shared_domain
@request.host = "example3.com"
get :set_cookie_with_domains
assert_response :success
assert_cookie_header "user_name=rizwanreza; domain=.example3.com; path=/"
assert_cookie_header "user_name=rizwanreza; domain=.example3.com; path=/; SameSite=Lax"
end

def test_deletings_cookie_with_several_preset_domains_using_one_of_these_domains
@request.host = "example2.com"
request.cookies[:user_name] = "Joe"
get :delete_cookie_with_domains
assert_response :success
assert_cookie_header "user_name=; domain=example2.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; domain=example2.com; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
end

def test_deletings_cookie_with_several_preset_domains_using_other_domain
@request.host = "other-domain.com"
request.cookies[:user_name] = "Joe"
get :delete_cookie_with_domains
assert_response :success
assert_cookie_header "user_name=; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000"
assert_cookie_header "user_name=; path=/; max-age=0; expires=Thu, 01 Jan 1970 00:00:00 -0000; SameSite=Lax"
end

def test_cookies_hash_is_indifferent_access
Expand All @@ -1062,7 +1088,7 @@ def test_setting_request_cookies_is_indifferent_access

def test_cookies_retained_across_requests
get :symbol_key
assert_cookie_header "user_name=david; path=/"
assert_cookie_header "user_name=david; path=/; SameSite=Lax"
assert_equal "david", cookies[:user_name]

get :noop
Expand Down Expand Up @@ -1181,7 +1207,7 @@ def test_encrypted_cookie_with_expires_set_relatively
def test_vanilla_cookie_with_expires_set_relatively
travel_to Time.utc(2017, 8, 15) do
get :cookie_expires_in_two_hours
assert_cookie_header "user_name=assain; path=/; expires=Tue, 15 Aug 2017 02:00:00 -0000"
assert_cookie_header "user_name=assain; path=/; expires=Tue, 15 Aug 2017 02:00:00 -0000; SameSite=Lax"
end
end

Expand Down
1 change: 1 addition & 0 deletions railties/lib/rails/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ def env_config
"action_dispatch.cookies_serializer" => config.action_dispatch.cookies_serializer,
"action_dispatch.cookies_digest" => config.action_dispatch.cookies_digest,
"action_dispatch.cookies_rotations" => config.action_dispatch.cookies_rotations,
"action_dispatch.cookies_same_site_protection" => config.action_dispatch.cookies_same_site_protection,
"action_dispatch.use_cookies_with_metadata" => config.action_dispatch.use_cookies_with_metadata,
"action_dispatch.content_security_policy" => config.content_security_policy,
"action_dispatch.content_security_policy_report_only" => config.content_security_policy_report_only,
Expand Down
4 changes: 4 additions & 0 deletions railties/lib/rails/application/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ def load_defaults(target_version)
if respond_to?(:active_job)
active_job.skip_after_callbacks_if_terminated = true
end

if respond_to?(:action_dispatch)
action_dispatch.cookies_same_site_protection = :lax
end
else
raise "Unknown version #{target_version.to_s.inspect}"
end
Expand Down
4 changes: 2 additions & 2 deletions railties/lib/rails/generators/rails/app/app_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ def config_when_updating
end

if options[:api]
unless cookie_serializer_config_exist
remove_file "config/initializers/cookies_serializer.rb"
unless cookies_config_exist
remove_file "config/initializers/cookies.rb"
end

unless csp_config_exist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@
# Rails.application.config.active_storage.track_variants = true

# Rails.application.config.active_job.skip_after_callbacks_if_terminated = true

# Specify cookies SameSite protection level: either :none, :lax, or :strict.
#
# This change is not backwards compatible with earlier Rails versions.
# It's best enabled when your entire app is migrated and stable on 6.1.
# Rails.application.config.action_dispatch.cookies_same_site_protection = :lax

0 comments on commit 7ccaa12

Please sign in to comment.