From f2c66ce392016efd49816cd6fdc9f174271a0321 Mon Sep 17 00:00:00 2001 From: Andrew Kowpak Date: Fri, 11 Mar 2022 10:53:39 -0500 Subject: [PATCH] Allow CSRF tokens to be stored outside of session --- actionpack/CHANGELOG.md | 8 + .../metal/request_forgery_protection.rb | 140 ++++++++- actionpack/lib/action_controller/test_case.rb | 9 +- .../lib/action_dispatch/http/request.rb | 5 + .../middleware/session/abstract_store.rb | 5 + .../lib/action_dispatch/request/session.rb | 10 + .../request_forgery_protection_test.rb | 266 ++++++++++++++++-- .../test/dispatch/request/session_test.rb | 42 +++ .../session/abstract_secure_store_test.rb | 4 + .../dispatch/session/abstract_store_test.rb | 4 + 10 files changed, 456 insertions(+), 37 deletions(-) diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 658f9b9519..1ea4689b8a 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add the ability to use custom logic for storing and retrieving CSRF tokens. + + By default, the token will be stored in the session. Custom classes can be + defined to specify arbitrary behaviour, but the ability to store them in + encrypted cookies is built in. + + *Andrew Kowpak* + * Make ActionController::Parameters#values cast nested hashes into parameters. *Gannon McGibbon* diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index c648742367..809723d386 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -55,6 +55,8 @@ module ActionController # :nodoc: # Learn more about CSRF attacks and securing your application in the # {Ruby on Rails Security Guide}[https://guides.rubyonrails.org/security.html]. module RequestForgeryProtection + CSRF_TOKEN = "action_controller.csrf_token" + extend ActiveSupport::Concern include AbstractController::Helpers @@ -90,6 +92,10 @@ module ActionController # :nodoc: config_accessor :default_protect_from_forgery self.default_protect_from_forgery = false + # The strategy to use for storing and retrieving CSRF tokens. + config_accessor :csrf_token_storage_strategy + self.csrf_token_storage_strategy = SessionStore.new + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -140,11 +146,39 @@ module ActionController # :nodoc: # class ApplicationController < ActionController:x:Base # protect_from_forgery with: CustomStrategy # end + # * :store - Set the strategy to store and retrieve CSRF tokens. + # + # Built-in session token strategies are: + # * :session - Store the CSRF token in the session. Used as default if :store option is not specified. + # * :cookie - Store the CSRF token in an encrypted cookie. + # + # You can also implement custom strategy classes for CSRF token storage: + # + # class CustomStore + # def fetch(request) + # # Return the token from a custom location + # end + # + # def store(request, csrf_token) + # # Store the token in a custom location + # end + # + # def reset(request) + # # Delete the stored session token + # end + # end + # + # class ApplicationController < ActionController:x:Base + # protect_from_forgery store: CustomStore.new + # end def protect_from_forgery(options = {}) options = options.reverse_merge(prepend: false) self.forgery_protection_strategy = protection_method_class(options[:with] || :null_session) self.request_forgery_protection_token ||= :authenticity_token + + self.csrf_token_storage_strategy = storage_strategy(options[:store] || SessionStore.new) + before_action :verify_authenticity_token, options append_after_action :verify_same_origin_request end @@ -173,6 +207,22 @@ module ActionController # :nodoc: raise ArgumentError, "Invalid request forgery protection method, use :null_session, :exception, :reset_session, or a custom forgery protection class." end end + + def storage_strategy(name) + case name + when :session + SessionStore.new + when :cookie + CookieStore.new(:csrf_token) + else + return name if is_storage_strategy?(name) + raise ArgumentError, "Invalid CSRF token storage strategy, use :session, :cookie, or a custom CSRF token storage class." + end + end + + def is_storage_strategy?(object) + object.respond_to?(:fetch) && object.respond_to?(:store) + end end module ProtectionMethods @@ -240,6 +290,63 @@ module ActionController # :nodoc: end end + class SessionStore + def fetch(request) + request.session[:_csrf_token] + end + + def store(request, csrf_token) + request.session[:_csrf_token] = csrf_token + end + + def reset(request) + request.session.delete(:_csrf_token) + end + end + + class CookieStore + def initialize(cookie = :csrf_token) + @cookie_name = cookie + end + + def fetch(request) + contents = request.cookie_jar.encrypted[@cookie_name] + return nil if contents.nil? + + value = JSON.parse(contents) + return nil unless value.dig("session_id", "public_id") == request.session.id_was&.public_id + + value["token"] + rescue JSON::ParserError + nil + end + + def store(request, csrf_token) + request.cookie_jar.encrypted.permanent[@cookie_name] = { + value: { + token: csrf_token, + session_id: request.session.id, + }.to_json, + httponly: true, + same_site: :lax, + } + end + + def reset(request) + request.cookie_jar.delete(@cookie_name) + end + end + + def reset_csrf_token(request) # :doc: + request.env.delete(CSRF_TOKEN) + csrf_token_storage_strategy.reset(request) + end + + def commit_csrf_token(request) # :doc: + csrf_token = request.env[CSRF_TOKEN] + csrf_token_storage_strategy.store(request, csrf_token) unless csrf_token.nil? + end + private # The actual before_action that is used to verify the CSRF token. # Don't override this directly. Provide your own forgery protection @@ -341,20 +448,20 @@ module ActionController # :nodoc: # Creates the authenticity token for the current request. def form_authenticity_token(form_options: {}) # :doc: - masked_authenticity_token(session, form_options: form_options) + masked_authenticity_token(form_options: form_options) end # Creates a masked version of the authenticity token that varies # on each request. The masking is used to mitigate SSL attacks # like BREACH. - def masked_authenticity_token(session, form_options: {}) + def masked_authenticity_token(form_options: {}) action, method = form_options.values_at(:action, :method) raw_token = if per_form_csrf_tokens && action && method action_path = normalize_action_path(action) - per_form_csrf_token(session, action_path, method) + per_form_csrf_token(nil, action_path, method) else - global_csrf_token(session) + global_csrf_token end mask_token(raw_token) @@ -382,14 +489,14 @@ module ActionController # :nodoc: # This is actually an unmasked token. This is expected if # you have just upgraded to masked tokens, but should stop # happening shortly after installing this gem. - compare_with_real_token masked_token, session + compare_with_real_token masked_token elsif masked_token.length == AUTHENTICITY_TOKEN_LENGTH * 2 csrf_token = unmask_token(masked_token) - compare_with_global_token(csrf_token, session) || - compare_with_real_token(csrf_token, session) || - valid_per_form_csrf_token?(csrf_token, session) + compare_with_global_token(csrf_token) || + compare_with_real_token(csrf_token) || + valid_per_form_csrf_token?(csrf_token) else false # Token is malformed. end @@ -410,15 +517,15 @@ module ActionController # :nodoc: encode_csrf_token(masked_token) end - def compare_with_real_token(token, session) # :doc: + def compare_with_real_token(token, session = nil) # :doc: ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, real_csrf_token(session)) end - def compare_with_global_token(token, session) # :doc: + def compare_with_global_token(token, session = nil) # :doc: ActiveSupport::SecurityUtils.fixed_length_secure_compare(token, global_csrf_token(session)) end - def valid_per_form_csrf_token?(token, session) # :doc: + def valid_per_form_csrf_token?(token, session = nil) # :doc: if per_form_csrf_tokens correct_token = per_form_csrf_token( session, @@ -432,9 +539,12 @@ module ActionController # :nodoc: end end - def real_csrf_token(session) # :doc: - session[:_csrf_token] ||= generate_csrf_token - decode_csrf_token(session[:_csrf_token]) + def real_csrf_token(_session = nil) # :doc: + csrf_token = request.env.fetch(CSRF_TOKEN) do + request.env[CSRF_TOKEN] = csrf_token_storage_strategy.fetch(request) || generate_csrf_token + end + + decode_csrf_token(csrf_token) end def per_form_csrf_token(session, action_path, method) # :doc: @@ -444,7 +554,7 @@ module ActionController # :nodoc: GLOBAL_CSRF_TOKEN_IDENTIFIER = "!real_csrf_token" private_constant :GLOBAL_CSRF_TOKEN_IDENTIFIER - def global_csrf_token(session) # :doc: + def global_csrf_token(session = nil) # :doc: csrf_token_hmac(session, GLOBAL_CSRF_TOKEN_IDENTIFIER) end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index e1f0ac788a..e643f74f61 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -182,11 +182,12 @@ module ActionController class TestSession < Rack::Session::Abstract::PersistedSecure::SecureSessionHash # :nodoc: DEFAULT_OPTIONS = Rack::Session::Abstract::Persisted::DEFAULT_OPTIONS - def initialize(session = {}) + def initialize(session = {}, id = Rack::Session::SessionId.new(SecureRandom.hex(16))) super(nil, nil) - @id = Rack::Session::SessionId.new(SecureRandom.hex(16)) + @id = id @data = stringify_keys(session) @loaded = true + @initially_empty = @data.empty? end def exists? @@ -218,6 +219,10 @@ module ActionController true end + def id_was + @id + end + private def load! @id diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index dc854f414c..ea98d244d0 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -358,6 +358,7 @@ module ActionDispatch def reset_session session.destroy + controller_instance.reset_csrf_token(self) if controller_instance.respond_to?(:reset_csrf_token) end def session=(session) # :nodoc: @@ -429,6 +430,10 @@ module ActionDispatch "#<#{self.class.name} #{method} #{original_url.dump} for #{remote_ip}>" end + def commit_csrf_token + controller_instance.commit_csrf_token(self) if controller_instance.respond_to?(:commit_csrf_token) + end + private def check_method(name) HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS[0...-1].join(', ')}, and #{HTTP_METHODS[-1]}") diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 90226c7363..c76bc49e72 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -67,6 +67,11 @@ module ActionDispatch end module SessionObject # :nodoc: + def commit_session(req, res) + req.commit_csrf_token + super(req, res) + end + def prepare_session(req) Request::Session.create(self, req, @default_options) end diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb index b9ad64c640..fa4502f08c 100644 --- a/actionpack/lib/action_dispatch/request/session.rb +++ b/actionpack/lib/action_dispatch/request/session.rb @@ -78,6 +78,8 @@ module ActionDispatch @loaded = false @exists = nil # We haven't checked yet. @enabled = enabled + @id_was = nil + @id_was_initialized = false end def id @@ -241,6 +243,11 @@ module ActionDispatch to_hash.each(&block) end + def id_was + load_for_read! + @id_was + end + private def load_for_read! load! if !loaded? && exists? @@ -260,10 +267,13 @@ module ActionDispatch def load! if enabled? + @id_was_initialized = true unless exists? id, session = @by.load_session @req options[:id] = id @delegate.replace(session.stringify_keys) + @id_was = id unless @id_was_initialized end + @id_was_initialized = true @loaded = true end end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 87be981852..8152946f27 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -195,6 +195,50 @@ class SkipProtectionWhenUnprotectedController < ActionController::Base skip_forgery_protection end +class CookieCsrfTokenStorageStrategyController < ActionController::Base + include RequestForgeryProtectionActions + + after_action :commit_token, only: :cookie + + protect_from_forgery only: %w(index meta same_origin_js negotiate_same_origin), with: :exception, store: :cookie + + def reset + reset_csrf_token(request) + head :ok + end + + def cookie + render inline: "<%= csrf_meta_tags %>" + end + + private + def commit_token + request.commit_csrf_token + end +end + +class CustomCsrfTokenStorageStrategyController < ActionController::Base + include RequestForgeryProtectionActions + + class CustomStrategy + def fetch(request) + request.env[:custom_storage] + end + + def store(request, csrf_token) + request.env[:custom_storage] = csrf_token + end + + def reset(request) + request.env[:custom_storage] = nil + end + end + + protect_from_forgery only: %w(index meta same_origin_js negotiate_same_origin), + with: :reset_session, + store: CustomStrategy.new +end + # common test methods module RequestForgeryProtectionTests def setup @@ -394,7 +438,7 @@ module RequestForgeryProtectionTests end def test_should_allow_post_with_token - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked { post :index, params: { custom_authenticity_token: @token } } end @@ -403,60 +447,60 @@ module RequestForgeryProtectionTests 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 + initialize_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_patch_with_token - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked { patch :index, params: { custom_authenticity_token: @token } } end end def test_should_allow_put_with_token - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked { put :index, params: { custom_authenticity_token: @token } } end end def test_should_allow_delete_with_token - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked { delete :index, params: { custom_authenticity_token: @token } } end end def test_should_allow_post_with_token_in_header - session[:_csrf_token] = @token + initialize_csrf_token @request.env["HTTP_X_CSRF_TOKEN"] = @token assert_not_blocked { post :index } end def test_should_allow_delete_with_token_in_header - session[:_csrf_token] = @token + initialize_csrf_token @request.env["HTTP_X_CSRF_TOKEN"] = @token assert_not_blocked { delete :index } end def test_should_allow_patch_with_token_in_header - session[:_csrf_token] = @token + initialize_csrf_token @request.env["HTTP_X_CSRF_TOKEN"] = @token assert_not_blocked { patch :index } end def test_should_allow_put_with_token_in_header - session[:_csrf_token] = @token + initialize_csrf_token @request.env["HTTP_X_CSRF_TOKEN"] = @token assert_not_blocked { put :index } end def test_should_allow_post_with_origin_checking_and_correct_origin forgery_protection_origin_check do - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked do @request.set_header "HTTP_ORIGIN", "http://test.host" @@ -468,7 +512,7 @@ module RequestForgeryProtectionTests def test_should_allow_post_with_origin_checking_and_no_origin forgery_protection_origin_check do - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_not_blocked do post :index, params: { custom_authenticity_token: @token } @@ -479,7 +523,7 @@ module RequestForgeryProtectionTests def test_should_raise_for_post_with_null_origin forgery_protection_origin_check do - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do exception = assert_raises(ActionController::InvalidAuthenticityToken) do @request.set_header "HTTP_ORIGIN", "null" @@ -496,7 +540,7 @@ module RequestForgeryProtectionTests ActionController::Base.logger = logger forgery_protection_origin_check do - session[:_csrf_token] = @token + initialize_csrf_token @controller.stub :form_authenticity_token, @token do assert_blocked do @request.set_header "HTTP_ORIGIN", "http://bad.host" @@ -513,6 +557,7 @@ module RequestForgeryProtectionTests ActionController::Base.logger = old_logger end + def test_should_warn_on_missing_csrf_token old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new @@ -598,7 +643,7 @@ module RequestForgeryProtectionTests # Allow non-GET requests since GET is all a remote