diff options
| -rw-r--r-- | actionpack/lib/action_dispatch.rb | 2 | ||||
| -rwxr-xr-x | actionpack/lib/action_dispatch/http/request.rb | 31 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb | 25 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/remote_ip.rb | 51 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/railtie.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/dispatch/request_test.rb | 12 | ||||
| -rw-r--r-- | railties/lib/rails/configuration.rb | 5 | ||||
| -rw-r--r-- | railties/lib/rails/log_subscriber.rb | 1 | ||||
| -rw-r--r-- | railties/test/application/middleware_stack_defaults_test.rb | 53 |
9 files changed, 118 insertions, 63 deletions
diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 1abb283b11..dfb8919561 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -42,13 +42,13 @@ module ActionDispatch end autoload_under 'middleware' do - autoload :BlockUntrustedIps autoload :Callbacks autoload :Cascade autoload :Cookies autoload :Flash autoload :Head autoload :ParamsParser + autoload :RemoteIp autoload :Rescue autoload :ShowExceptions autoload :Static diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7a17023ed2..56a2b9bf6a 100755 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -119,36 +119,7 @@ module ActionDispatch # delimited list in the case of multiple chained proxies; the last # address which is not trusted is the originating IP. def remote_ip - remote_addr_list = @env['REMOTE_ADDR'] && @env['REMOTE_ADDR'].scan(/[^,\s]+/) - - unless remote_addr_list.blank? - not_trusted_addrs = remote_addr_list.reject {|addr| addr =~ TRUSTED_PROXIES || addr =~ ActionController::Base.trusted_proxies} - return not_trusted_addrs.first unless not_trusted_addrs.empty? - end - remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',') - - if @env.include? 'HTTP_CLIENT_IP' - if ActionController::Base.ip_spoofing_check && remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP']) - # We don't know which came from the proxy, and which from the user - raise ActionController::ActionControllerError.new <<EOM -IP spoofing attack?! -HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect} -HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect} -EOM - end - - return @env['HTTP_CLIENT_IP'] - end - - if remote_ips - while remote_ips.size > 1 && (TRUSTED_PROXIES =~ remote_ips.last.strip || ActionController::Base.trusted_proxies =~ remote_ips.last.strip) - remote_ips.pop - end - - return remote_ips.last.strip - end - - @env['REMOTE_ADDR'] + (@env["action_dispatch.remote_ip"] || ip).to_s end # Returns the lowercase name of the HTTP server software. diff --git a/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb b/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb deleted file mode 100644 index 8aed0c45a6..0000000000 --- a/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb +++ /dev/null @@ -1,25 +0,0 @@ -module ActionDispatch - class BlockUntrustedIps - class SpoofAttackError < StandardError ; end - - def initialize(app) - @app = app - end - - def call(env) - if @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_CLIENT_IP'] - remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',') - - unless remote_ips.include?(@env['HTTP_CLIENT_IP']) - http_client_ip = @env['HTTP_CLIENT_IP'].inspect - http_forwarded_for = @env['HTTP_X_FORWARDED_FOR'].inspect - - raise SpoofAttackError, "IP spoofing attack?!\n " \ - "HTTP_CLIENT_IP=#{http_client_ip}\n HTTP_X_FORWARDED_FOR=http_forwarded_for" - end - end - - @app.call(env) - end - end -end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb new file mode 100644 index 0000000000..c7d710b98e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb @@ -0,0 +1,51 @@ +module ActionDispatch + class RemoteIp + class IpSpoofAttackError < StandardError ; end + + class RemoteIpGetter + def initialize(env, check_ip_spoofing, trusted_proxies) + @env = env + @check_ip_spoofing = check_ip_spoofing + @trusted_proxies = trusted_proxies + end + + def remote_addrs + @remote_addrs ||= begin + list = @env['REMOTE_ADDR'] ? @env['REMOTE_ADDR'].split(/[,\s]+/) : [] + list.reject { |addr| addr =~ @trusted_proxies } + end + end + + def to_s + return remote_addrs.first if remote_addrs.any? + + forwarded_ips = @env['HTTP_X_FORWARDED_FOR'] ? @env['HTTP_X_FORWARDED_FOR'].strip.split(/[,\s]+/) : [] + + if client_ip = @env['HTTP_CLIENT_IP'] + if @check_ip_spoofing && !forwarded_ips.include?(client_ip) + # We don't know which came from the proxy, and which from the user + raise IpSpoofAttackError, "IP spoofing attack?!" \ + "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}" \ + "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}" + end + return client_ip + end + + return forwarded_ips.reject { |ip| ip =~ @trusted_proxies }.last || @env["REMOTE_ADDR"] + end + end + + def initialize(app, check_ip_spoofing = true, trusted_proxies = nil) + @app = app + @check_ip_spoofing = check_ip_spoofing + regex = '(^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.)' + regex << "|(#{trusted_proxies})" if trusted_proxies + @trusted_proxies = Regexp.new(regex, "i") + end + + def call(env) + env["action_dispatch.remote_ip"] = RemoteIpGetter.new(env, @check_ip_spoofing, @trusted_proxies) + @app.call(env) + end + end +end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 79e9464337..e486bd4079 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -6,6 +6,7 @@ module ActionDispatch railtie_name :action_dispatch config.action_dispatch.x_sendfile_header = "X-Sendfile" + config.action_dispatch.ip_spoofing_check = true # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 78200ca17e..3a07185586 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -42,7 +42,7 @@ class RequestTest < ActiveSupport::TestCase request = stub_request 'HTTP_X_FORWARDED_FOR' => '1.1.1.1', 'HTTP_CLIENT_IP' => '2.2.2.2' - e = assert_raise(ActionController::ActionControllerError) { + e = assert_raise(ActionDispatch::RemoteIp::IpSpoofAttackError) { request.remote_ip } assert_match /IP spoofing attack/, e.message @@ -54,18 +54,17 @@ class RequestTest < ActiveSupport::TestCase # example is WAP. Since the cellular network is not IP based, it's a # leap of faith to assume that their proxies are ever going to set the # HTTP_CLIENT_IP/HTTP_X_FORWARDED_FOR headers properly. - ActionController::Base.ip_spoofing_check = false request = stub_request 'HTTP_X_FORWARDED_FOR' => '1.1.1.1', - 'HTTP_CLIENT_IP' => '2.2.2.2' + 'HTTP_CLIENT_IP' => '2.2.2.2', + :ip_spoofing_check => false assert_equal '2.2.2.2', request.remote_ip - ActionController::Base.ip_spoofing_check = true request = stub_request 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, 9.9.9.9' assert_equal '9.9.9.9', request.remote_ip end test "remote ip with user specified trusted proxies" do - ActionController::Base.trusted_proxies = /^67\.205\.106\.73$/i + @trusted_proxies = /^67\.205\.106\.73$/i request = stub_request 'REMOTE_ADDR' => '67.205.106.73', 'HTTP_X_FORWARDED_FOR' => '3.4.5.6' @@ -429,6 +428,9 @@ class RequestTest < ActiveSupport::TestCase protected def stub_request(env = {}) + ip_spoofing_check = env.key?(:ip_spoofing_check) ? env.delete(:ip_spoofing_check) : true + ip_app = ActionDispatch::RemoteIp.new(Proc.new { }, ip_spoofing_check, @trusted_proxies) + ip_app.call(env) ActionDispatch::Request.new(env) end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index a1e901f04f..a26978ef92 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -84,11 +84,12 @@ module Rails middleware.use('::Rack::Runtime') middleware.use('::Rails::Rack::Logger') middleware.use('::ActionDispatch::ShowExceptions', lambda { consider_all_requests_local }) + middleware.use("::ActionDispatch::RemoteIp", lambda { action_dispatch.ip_spoofing_check }, lambda { action_dispatch.trusted_proxies }) middleware.use('::Rack::Sendfile', lambda { action_dispatch.x_sendfile_header }) middleware.use('::ActionDispatch::Callbacks', lambda { !cache_classes }) middleware.use('::ActionDispatch::Cookies') - middleware.use(lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options }) - middleware.use('::ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store }) + middleware.use(lambda { action_controller.session_store }, lambda { action_controller.session_options }) + middleware.use('::ActionDispatch::Flash', :if => lambda { action_controller.session_store }) middleware.use(lambda { metal_loader.build_middleware(metals) }, :if => lambda { metal_loader.metals.any? }) middleware.use('ActionDispatch::ParamsParser') middleware.use('::Rack::MethodOverride') diff --git a/railties/lib/rails/log_subscriber.rb b/railties/lib/rails/log_subscriber.rb index 0fbc19d89c..42697d2e32 100644 --- a/railties/lib/rails/log_subscriber.rb +++ b/railties/lib/rails/log_subscriber.rb @@ -87,6 +87,7 @@ module Rails %w(info debug warn error fatal unknown).each do |level| class_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{level}(*args, &block) + return unless logger logger.#{level}(*args, &block) end METHOD diff --git a/railties/test/application/middleware_stack_defaults_test.rb b/railties/test/application/middleware_stack_defaults_test.rb new file mode 100644 index 0000000000..94151a90da --- /dev/null +++ b/railties/test/application/middleware_stack_defaults_test.rb @@ -0,0 +1,53 @@ +require 'isolation/abstract_unit' + +class MiddlewareStackDefaultsTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + boot_rails + require "rails" + require "action_controller/railtie" + + Object.const_set(:MyApplication, Class.new(Rails::Application)) + MyApplication.class_eval do + config.action_controller.session = { :key => "_myapp_session", :secret => "OMG A SEKRET" * 10 } + end + end + + def remote_ip(env = {}) + remote_ip = nil + env = Rack::MockRequest.env_for("/").merge(env).merge('action_dispatch.show_exceptions' => false) + + endpoint = Proc.new do |e| + remote_ip = ActionDispatch::Request.new(e).remote_ip + [200, {}, ["Hello"]] + end + + out = MyApplication.middleware.build(endpoint).call(env) + remote_ip + end + + test "remote_ip works" do + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1") + end + + test "checks IP spoofing by default" do + assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do + remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "can disable IP spoofing check" do + MyApplication.config.action_dispatch.ip_spoofing_check = false + + assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do + assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2") + end + end + + test "the user can set trusted proxies" do + MyApplication.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/ + + assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1") + end +end |