From 85783534fcf1baefa5b502a2bfee235ae6d612d7 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Wed, 25 Nov 2015 15:06:12 -0700 Subject: Add option to verify Origin header in CSRF checks --- .../metal/request_forgery_protection.rb | 30 ++++++++++++++- actionpack/lib/action_dispatch/http/request.rb | 4 +- .../controller/request_forgery_protection_test.rb | 45 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 64f6f7cf51..836ed892dc 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -77,6 +77,10 @@ module ActionController #:nodoc: config_accessor :log_warning_on_csrf_failure self.log_warning_on_csrf_failure = true + # Controls whether the Origin header is checked in addition to the CSRF token. + config_accessor :forgery_protection_origin_check + self.forgery_protection_origin_check = false + helper_method :form_authenticity_token helper_method :protect_against_forgery? end @@ -257,8 +261,19 @@ module ActionController #:nodoc: # * Does the X-CSRF-Token header match the form_authenticity_token def verified_request? !protect_against_forgery? || request.get? || request.head? || - valid_authenticity_token?(session, form_authenticity_param) || - valid_authenticity_token?(session, request.headers['X-CSRF-Token']) + (valid_request_origin? && any_authenticity_token_valid?) + end + + # Checks if any of the authenticity tokens from the request are valid. + def any_authenticity_token_valid? + request_authenticity_tokens.any? do |token| + valid_authenticity_token?(session, token) + end + end + + # Possible authenticity tokens sent in the request. + def request_authenticity_tokens + [form_authenticity_param, request.x_csrf_token] end # Sets the token value for the current session. @@ -336,5 +351,16 @@ module ActionController #:nodoc: def protect_against_forgery? allow_forgery_protection end + + # Checks if the request originated from the same origin by looking at the + # Origin header. + def valid_request_origin? + if forgery_protection_origin_check + # We accept blank origin headers because some user agents don't send it. + request.origin.nil? || request.origin == request.base_url + else + true + end + end end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index ea61ad0c02..0c8d0a5d14 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -36,8 +36,8 @@ module ActionDispatch HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM HTTP_NEGOTIATE HTTP_PRAGMA HTTP_CLIENT_IP - HTTP_X_FORWARDED_FOR HTTP_VERSION - HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST + HTTP_X_FORWARDED_FOR HTTP_ORIGIN HTTP_VERSION + HTTP_X_CSRF_TOKEN HTTP_X_REQUEST_ID HTTP_X_FORWARDED_HOST SERVER_ADDR ].freeze diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 94ffbe3cd0..2a3704c300 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -304,6 +304,41 @@ module RequestForgeryProtectionTests 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 + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://test.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_allow_post_with_origin_checking_and_no_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_not_blocked do + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + + def test_should_block_post_with_origin_checking_and_wrong_origin + forgery_protection_origin_check do + session[:_csrf_token] = @token + @controller.stub :form_authenticity_token, @token do + assert_blocked do + @request.set_header 'HTTP_ORIGIN', 'http://bad.host' + post :index, params: { custom_authenticity_token: @token } + end + end + end + end + def test_should_warn_on_missing_csrf_token old_logger = ActionController::Base.logger logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new @@ -405,6 +440,16 @@ module RequestForgeryProtectionTests def assert_cross_origin_not_blocked assert_not_blocked { yield } end + + def forgery_protection_origin_check + old_setting = ActionController::Base.forgery_protection_origin_check + ActionController::Base.forgery_protection_origin_check = true + begin + yield + ensure + ActionController::Base.forgery_protection_origin_check = old_setting + end + end end # OK let's get our test on -- cgit v1.2.3