From 64bb514358d01106ddfcb20849581dc8da344e30 Mon Sep 17 00:00:00 2001 From: Scott Blum Date: Sun, 29 Mar 2020 00:07:09 -0400 Subject: [PATCH 1/3] Accept and default to base64_urlsafe CSRF tokens (#18496) Base64 strict-encoded CSRF tokens are not inherently websafe, which makes them difficult to deal with. For example, the common practice of sending the CSRF token to a browser in a client-readable cookie does not work properly out of the box: the value has to be url-encoded and decoded to survive transport. Now, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently safe to transport. Validation accepts both urlsafe tokens, and strict-encoded tokens for backwards compatibility. --- actionpack/CHANGELOG.md | 13 ++++++++++++ .../metal/request_forgery_protection.rb | 8 +++---- .../request_forgery_protection_test.rb | 21 ++++++++++++------- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 8e2751aedb358..3f85e095c63e0 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,16 @@ +* Accept and default to base64_urlsafe CSRF tokens. + + Base64 strict-encoded CSRF tokens are not inherently websafe, which makes + them difficult to deal with. For example, the common practice of sending + the CSRF token to a browser in a client-readable cookie does not work properly + out of the box: the value has to be url-encoded and decoded to survive transport. + + Now, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently safe + to transport. Validation accepts both urlsafe tokens, and strict-encoded tokens + for backwards compatibility. + + *Scott Blum* + * Signed and encrypted cookies can now store `false` as their value when `action_dispatch.use_cookies_with_metadata` is enabled. diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 8123170533346..a78b1f7154743 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -329,7 +329,7 @@ def valid_authenticity_token?(session, encoded_masked_token) # :doc: end begin - masked_token = Base64.strict_decode64(encoded_masked_token) + masked_token = Base64.urlsafe_decode64(encoded_masked_token) rescue ArgumentError # encoded_masked_token is invalid Base64 return false end @@ -367,7 +367,7 @@ def mask_token(raw_token) # :doc: one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH) encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token) masked_token = one_time_pad + encrypted_csrf_token - Base64.strict_encode64(masked_token) + Base64.urlsafe_encode64(masked_token, padding: false) end def compare_with_real_token(token, session) # :doc: @@ -393,8 +393,8 @@ def valid_per_form_csrf_token?(token, session) # :doc: end def real_csrf_token(session) # :doc: - session[:_csrf_token] ||= SecureRandom.base64(AUTHENTICITY_TOKEN_LENGTH) - Base64.strict_decode64(session[:_csrf_token]) + session[:_csrf_token] ||= SecureRandom.urlsafe_base64(AUTHENTICITY_TOKEN_LENGTH, padding: false) + Base64.urlsafe_decode64(session[:_csrf_token]) end def per_form_csrf_token(session, action_path, method) # :doc: diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 10de9bcc3e533..dbb2caa3b2e58 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -175,7 +175,7 @@ class SkipProtectionController < ActionController::Base # common test methods module RequestForgeryProtectionTests def setup - @token = Base64.strict_encode64("railstestrailstestrailstestrails") + @token = Base64.urlsafe_encode64("railstestrailstestrailstestrails") @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token ActionController::Base.request_forgery_protection_token = :custom_authenticity_token end @@ -377,6 +377,13 @@ def test_should_allow_post_with_token end end + def test_should_allow_post_with_strict_encoded_token + session[:_csrf_token] = Base64.strict_encode64("railstestrailstestrailstestrails") + @controller.stub :form_authenticity_token, @token do + assert_not_blocked { post :index, params: { custom_authenticity_token: @token } } + end + end + def test_should_allow_patch_with_token session[:_csrf_token] = @token @controller.stub :form_authenticity_token, @token do @@ -735,21 +742,21 @@ def setup end def test_should_not_render_form_with_token_tag - SecureRandom.stub :base64, @token do + SecureRandom.stub :urlsafe_base64, @token do get :index assert_select "form>div>input[name=?][value=?]", "authenticity_token", @token, false end end def test_should_not_render_button_to_with_token_tag - SecureRandom.stub :base64, @token do + SecureRandom.stub :urlsafe_base64, @token do get :show_button assert_select "form>div>input[name=?][value=?]", "authenticity_token", @token, false end end def test_should_allow_all_methods_without_token - SecureRandom.stub :base64, @token do + SecureRandom.stub :urlsafe_base64, @token do [:post, :patch, :put, :delete].each do |method| assert_nothing_raised { send(method, :index) } end @@ -757,7 +764,7 @@ def test_should_allow_all_methods_without_token end test "should not emit a csrf-token meta tag" do - SecureRandom.stub :base64, @token do + SecureRandom.stub :urlsafe_base64, @token do get :meta assert_predicate @response.body, :blank? end @@ -769,7 +776,7 @@ def setup super @old_logger = ActionController::Base.logger @logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new - @token = Base64.strict_encode64(SecureRandom.random_bytes(32)) + @token = Base64.urlsafe_encode64(SecureRandom.random_bytes(32)) @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token ActionController::Base.request_forgery_protection_token = @token end @@ -1029,7 +1036,7 @@ def assert_presence_and_fetch_form_csrf_token end def assert_matches_session_token_on_server(form_token, method = "post") - actual = @controller.send(:unmask_token, Base64.strict_decode64(form_token)) + actual = @controller.send(:unmask_token, Base64.urlsafe_decode64(form_token)) expected = @controller.send(:per_form_csrf_token, session, "/per_form_tokens/post_one", method) assert_equal expected, actual end From f35b4ae90ce45b85f5a2a839b91a710d092f61d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 11 Jun 2020 11:40:32 -0400 Subject: [PATCH 2/3] Merge pull request #39076 from etiennebarrie/upgrade-safe-urlsafe-csrf-tokens Upgrade-safe URL-safe CSRF tokens --- actionpack/CHANGELOG.md | 2 +- .../metal/request_forgery_protection.rb | 40 +++++++++++++++++-- .../request_forgery_protection_test.rb | 24 +++++++++-- guides/source/configuring.md | 2 + 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 3f85e095c63e0..fdacb35c03394 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -6,7 +6,7 @@ out of the box: the value has to be url-encoded and decoded to survive transport. Now, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently safe - to transport. Validation accepts both urlsafe tokens, and strict-encoded tokens + to transport. Validation accepts both urlsafe tokens, and strict-encoded tokens for backwards compatibility. *Scott Blum* diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index a78b1f7154743..3c7faaf1d1c94 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -90,6 +90,10 @@ module RequestForgeryProtection config_accessor :default_protect_from_forgery self.default_protect_from_forgery = false + # Controls whether URL-safe CSRF tokens are generated. + config_accessor :urlsafe_csrf_tokens, instance_writer: false + self.urlsafe_csrf_tokens = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -329,7 +333,7 @@ def valid_authenticity_token?(session, encoded_masked_token) # :doc: end begin - masked_token = Base64.urlsafe_decode64(encoded_masked_token) + masked_token = decode_csrf_token(encoded_masked_token) rescue ArgumentError # encoded_masked_token is invalid Base64 return false end @@ -367,7 +371,7 @@ def mask_token(raw_token) # :doc: one_time_pad = SecureRandom.random_bytes(AUTHENTICITY_TOKEN_LENGTH) encrypted_csrf_token = xor_byte_strings(one_time_pad, raw_token) masked_token = one_time_pad + encrypted_csrf_token - Base64.urlsafe_encode64(masked_token, padding: false) + encode_csrf_token(masked_token) end def compare_with_real_token(token, session) # :doc: @@ -393,8 +397,8 @@ def valid_per_form_csrf_token?(token, session) # :doc: end def real_csrf_token(session) # :doc: - session[:_csrf_token] ||= SecureRandom.urlsafe_base64(AUTHENTICITY_TOKEN_LENGTH, padding: false) - Base64.urlsafe_decode64(session[:_csrf_token]) + session[:_csrf_token] ||= generate_csrf_token + decode_csrf_token(session[:_csrf_token]) end def per_form_csrf_token(session, action_path, method) # :doc: @@ -462,5 +466,33 @@ def normalize_action_path(action_path) # :doc: uri = URI.parse(action_path) uri.path.chomp("/") end + + def generate_csrf_token # :nodoc: + if urlsafe_csrf_tokens + SecureRandom.urlsafe_base64(AUTHENTICITY_TOKEN_LENGTH, padding: false) + else + SecureRandom.base64(AUTHENTICITY_TOKEN_LENGTH) + end + end + + def encode_csrf_token(csrf_token) # :nodoc: + if urlsafe_csrf_tokens + Base64.urlsafe_encode64(csrf_token, padding: false) + else + Base64.strict_encode64(csrf_token) + end + end + + def decode_csrf_token(encoded_csrf_token) # :nodoc: + if urlsafe_csrf_tokens + Base64.urlsafe_decode64(encoded_csrf_token) + else + begin + Base64.strict_decode64(encoded_csrf_token) + rescue ArgumentError + Base64.urlsafe_decode64(encoded_csrf_token) + end + end + end end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index dbb2caa3b2e58..1ad48b5cf69f3 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -175,12 +175,15 @@ class SkipProtectionController < ActionController::Base # common test methods module RequestForgeryProtectionTests def setup + @old_urlsafe_csrf_tokens = ActionController::Base.urlsafe_csrf_tokens + ActionController::Base.urlsafe_csrf_tokens = true @token = Base64.urlsafe_encode64("railstestrailstestrailstestrails") @old_request_forgery_protection_token = ActionController::Base.request_forgery_protection_token ActionController::Base.request_forgery_protection_token = :custom_authenticity_token end def teardown + ActionController::Base.urlsafe_csrf_tokens = @old_urlsafe_csrf_tokens ActionController::Base.request_forgery_protection_token = @old_request_forgery_protection_token end @@ -378,10 +381,25 @@ def test_should_allow_post_with_token end def test_should_allow_post_with_strict_encoded_token - session[:_csrf_token] = Base64.strict_encode64("railstestrailstestrailstestrails") - @controller.stub :form_authenticity_token, @token do - assert_not_blocked { post :index, params: { custom_authenticity_token: @token } } + token_length = (ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH * 4.0 / 3).ceil + token_including_url_unsafe_chars = "+/".ljust(token_length, "A") + session[:_csrf_token] = token_including_url_unsafe_chars + @controller.stub :form_authenticity_token, token_including_url_unsafe_chars do + assert_not_blocked { post :index, params: { custom_authenticity_token: token_including_url_unsafe_chars } } + end + end + + def test_should_allow_post_with_urlsafe_token_when_migrating + config_before = ActionController::Base.urlsafe_csrf_tokens + ActionController::Base.urlsafe_csrf_tokens = false + token_length = (ActionController::RequestForgeryProtection::AUTHENTICITY_TOKEN_LENGTH * 4.0 / 3).ceil + token_including_url_safe_chars = "-_".ljust(token_length, "A") + session[:_csrf_token] = token_including_url_safe_chars + @controller.stub :form_authenticity_token, token_including_url_safe_chars do + assert_not_blocked { post :index, params: { custom_authenticity_token: token_including_url_safe_chars } } end + ensure + ActionController::Base.urlsafe_csrf_tokens = config_before end def test_should_allow_patch_with_token diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 61da951ab8805..29750bc97701d 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -486,6 +486,8 @@ The schema dumper adds two additional configuration options: * `config.action_controller.default_protect_from_forgery` determines whether forgery protection is added on `ActionController:Base`. This is false by default. +* `config.action_controller.urlsafe_csrf_tokens` configures whether generated CSRF tokens are URL-safe. Defaults to `false`. + * `config.action_controller.relative_url_root` can be used to tell Rails that you are [deploying to a subdirectory](configuring.html#deploy-to-a-subdirectory-relative-url-root). The default is `ENV['RAILS_RELATIVE_URL_ROOT']`. * `config.action_controller.permit_all_parameters` sets all the parameters for mass assignment to be permitted by default. The default value is `false`. From ef808bf1aad7a6d4d25afa3ed88f057c75c90b50 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 31 Mar 2021 12:59:19 +0900 Subject: [PATCH 3/3] Clarify how to upgrade apps from Rails 5.2.5 [ci skip] --- actionpack/CHANGELOG.md | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index fdacb35c03394..2d91ba1f8ba65 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,15 +1,22 @@ -* Accept and default to base64_urlsafe CSRF tokens. +* Accept base64_urlsafe CSRF tokens to make forward compatible. Base64 strict-encoded CSRF tokens are not inherently websafe, which makes them difficult to deal with. For example, the common practice of sending the CSRF token to a browser in a client-readable cookie does not work properly out of the box: the value has to be url-encoded and decoded to survive transport. - Now, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently safe - to transport. Validation accepts both urlsafe tokens, and strict-encoded tokens - for backwards compatibility. + In Rails 6.1, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently + safe to transport. Validation accepts both urlsafe tokens, and strict-encoded + tokens for backwards compatibility. - *Scott Blum* + In Rails 5.2.5, the CSRF token format is accidentally changed to urlsafe-encoded. + If you upgrade apps from 5.2.5, set the config `urlsafe_csrf_tokens = true`. + + ```ruby + Rails.application.config.action_controller.urlsafe_csrf_tokens = true + ``` + + *Scott Blum*, *Étienne Barrié* * Signed and encrypted cookies can now store `false` as their value when `action_dispatch.use_cookies_with_metadata` is enabled.