diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6d68c1ad07ad81e163de46b638263e3e9fcbf6cc..cdecfea384e5b34dd84ea4781538985b4253e939 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* + * Support rolling deploys for cookie serialization/encryption changes. In a distributed configuration like rolling update, users may observe diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index fa1586a917df50e3eb30b5b1002ec4dcdcfd4411..c5893e0a551e782cf95dde49ba392e402881c59f 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -328,7 +328,7 @@ def masked_authenticity_token(session, form_options: {}) # :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 # Checks the client's masked token to see if it matches the @@ -340,7 +340,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 @@ -392,8 +392,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 eea414ce3f87bb31d14fb8fbbe83b93ac45c1dad..89bbbeefbe90dec798aeb44a5b9378d91b227d28 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 @@ -1022,7 +1029,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