Merge pull request #41806 from kamipo/backport_39076_to_6-0-stable

[6-0-stable] Backport Upgrade-safe URL-safe CSRF tokens #39076

Author: kamipo

actionpack/CHANGELOG.md

@@ -1,3 +1,24 @@
+*   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.
+
+    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.
+
+    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é*
+
+
 ## Rails 5.2.5 (March 26, 2021) ##
 
 *   No changes.

actionpack/lib/action_controller/metal/request_forgery_protection.rb

@@ -92,6 +92,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
@@ -333,7 +337,7 @@ def valid_authenticity_token?(session, encoded_masked_token) # :doc:
         end
 
         begin
-          masked_token = Base64.strict_decode64(encoded_masked_token)
+          masked_token = decode_csrf_token(encoded_masked_token)
         rescue ArgumentError # encoded_masked_token is invalid Base64
           return false
         end
@@ -371,7 +375,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)
+        encode_csrf_token(masked_token)
       end
 
       def compare_with_real_token(token, session) # :doc:
@@ -397,8 +401,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] ||= generate_csrf_token
+        decode_csrf_token(session[:_csrf_token])
       end
 
       def per_form_csrf_token(session, action_path, method) # :doc:
@@ -461,5 +465,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

actionpack/test/controller/request_forgery_protection_test.rb

@@ -176,12 +176,15 @@ class SkipProtectionController < ActionController::Base
 # common test methods
 module RequestForgeryProtectionTests
   def setup
-    @token = Base64.strict_encode64("railstestrailstestrailstestrails")
+    @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,6 +381,28 @@ def test_should_allow_post_with_token
     end
   end
 
+  def test_should_allow_post_with_strict_encoded_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
     session[:_csrf_token] = @token
     @controller.stub :form_authenticity_token, @token do
@@ -722,29 +747,29 @@ 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
     end
   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
@@ -756,7 +781,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
@@ -1016,7 +1041,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

guides/source/configuring.md

@@ -428,6 +428,8 @@ The schema dumper adds one additional configuration option:
 
 * `config.action_controller.default_protect_from_forgery` determines whether forgery protection is added on `ActionController:Base`. This is false by default, but enabled when loading defaults for Rails 5.2.
 
+* `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`.