From 07ec8062e605ba4e9bd153e1d264b02ac4ab8a0f Mon Sep 17 00:00:00 2001 From: Genadi Samokovarov Date: Thu, 14 Jun 2018 11:09:00 +0300 Subject: Introduce a guard against DNS rebinding attacks The ActionDispatch::HostAuthorization is a new middleware that prevent against DNS rebinding and other Host header attacks. By default it is included only in the development environment with the following configuration: Rails.application.config.hosts = [ IPAddr.new("0.0.0.0/0"), # All IPv4 addresses. IPAddr.new("::/0"), # All IPv6 addresses. "localhost" # The localhost reserved domain. ] In other environments, `Rails.application.config.hosts` is empty and no Host header checks will be done. If you want to guard against header attacks on production, you have to manually permit the allowed hosts with: Rails.application.config.hosts << "product.com" The host of a request is checked against the hosts entries with the case operator (#===), which lets hosts support entries of type RegExp, Proc and IPAddr to name a few. Here is an example with a regexp. # Allow requests from subdomains like `www.product.com` and # `beta1.product.com`. Rails.application.config.hosts << /.*\.product\.com/ A special case is supported that allows you to permit all sub-domains: # Allow requests from subdomains like `www.product.com` and # `beta1.product.com`. Rails.application.config.hosts << ".product.com" --- actionpack/CHANGELOG.md | 10 ++ actionpack/lib/action_dispatch.rb | 2 + .../action_dispatch/middleware/debug_exceptions.rb | 43 +----- .../lib/action_dispatch/middleware/debug_view.rb | 50 +++++++ .../middleware/host_authorization.rb | 105 ++++++++++++++ .../templates/rescues/blocked_host.html.erb | 7 + .../templates/rescues/blocked_host.text.erb | 5 + .../test/dispatch/host_authorization_test.rb | 160 +++++++++++++++++++++ railties/CHANGELOG.md | 35 +++++ railties/lib/rails/application/configuration.rb | 4 +- .../rails/application/default_middleware_stack.rb | 2 + railties/lib/rails/info_controller.rb | 2 +- railties/lib/rails/mailers_controller.rb | 2 +- railties/test/application/middleware_test.rb | 14 +- railties/test/isolation/abstract_unit.rb | 2 + 15 files changed, 393 insertions(+), 50 deletions(-) create mode 100644 actionpack/lib/action_dispatch/middleware/debug_view.rb create mode 100644 actionpack/lib/action_dispatch/middleware/host_authorization.rb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb create mode 100644 actionpack/test/dispatch/host_authorization_test.rb diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 740c6db06f..066e92fec3 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,13 @@ +* Introduce ActionDispatch::HostAuthorization + + This is a new middleware that guards against DNS rebinding attacks by + white-listing the allowed hosts a request can be made to. + + Each host is checked with the case operator (`#===`) to support `RegExp`, + `Proc`, `IPAddr` and custom objects as host allowances. + + *Genadi Samokovarov* + * Raise an error on root route naming conflicts. Raises an ArgumentError when multiple root routes are defined in the diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 0822cdc0a6..a92d336214 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -49,11 +49,13 @@ module ActionDispatch end autoload_under "middleware" do + autoload :HostAuthorization autoload :RequestId autoload :Callbacks autoload :Cookies autoload :DebugExceptions autoload :DebugLocks + autoload :DebugView autoload :ExceptionWrapper autoload :Executor autoload :Flash diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 7669767ae3..eadb59173d 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -3,53 +3,14 @@ require "action_dispatch/http/request" require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" + require "action_view" require "action_view/base" -require "pp" - module ActionDispatch # This middleware is responsible for logging exceptions and # showing a debugging page in case the request is local. class DebugExceptions - RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) - - class DebugView < ActionView::Base - def debug_params(params) - clean_params = params.clone - clean_params.delete("action") - clean_params.delete("controller") - - if clean_params.empty? - "None" - else - PP.pp(clean_params, +"", 200) - end - end - - def debug_headers(headers) - if headers.present? - headers.inspect.gsub(",", ",\n") - else - "None" - end - end - - def debug_hash(object) - object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") - end - - def render(*) - logger = ActionView::Base.logger - - if logger && logger.respond_to?(:silence) - logger.silence { super } - else - super - end - end - end - cattr_reader :interceptors, instance_accessor: false, default: [] def self.register_interceptor(object = nil, &block) @@ -152,7 +113,7 @@ module ActionDispatch end def create_template(request, wrapper) - DebugView.new([RESCUES_TEMPLATE_PATH], + DebugView.new( request: request, exception_wrapper: wrapper, exception: wrapper.exception, diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb new file mode 100644 index 0000000000..ac12dc13a1 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "pp" + +require "action_view" +require "action_view/base" + +module ActionDispatch + class DebugView < ActionView::Base # :nodoc: + RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) + + def initialize(assigns) + super([RESCUES_TEMPLATE_PATH], assigns) + end + + def debug_params(params) + clean_params = params.clone + clean_params.delete("action") + clean_params.delete("controller") + + if clean_params.empty? + "None" + else + PP.pp(clean_params, +"", 200) + end + end + + def debug_headers(headers) + if headers.present? + headers.inspect.gsub(",", ",\n") + else + "None" + end + end + + def debug_hash(object) + object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") + end + + def render(*) + logger = ActionView::Base.logger + + if logger && logger.respond_to?(:silence) + logger.silence { super } + else + super + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/host_authorization.rb b/actionpack/lib/action_dispatch/middleware/host_authorization.rb new file mode 100644 index 0000000000..48f7c25216 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/host_authorization.rb @@ -0,0 +1,105 @@ +# frozen_string_literal: true + +require "action_dispatch/http/request" + +module ActionDispatch + # This middleware guards from DNS rebinding attacks by white-listing the + # hosts a request can be sent to. + # + # When a request comes to an unauthorized host, the +response_app+ + # application will be executed and rendered. If no +response_app+ is given, a + # default one will run, which responds with +403 Forbidden+. + class HostAuthorization + class Permissions # :nodoc: + def initialize(hosts) + @hosts = sanitize_hosts(hosts) + end + + def empty? + @hosts.empty? + end + + def allows?(host) + @hosts.any? do |allowed| + begin + allowed === host + rescue + # IPAddr#=== raises an error if you give it a hostname instead of + # IP. Treat similar errors as blocked access. + false + end + end + end + + private + + def sanitize_hosts(hosts) + Array(hosts).map do |host| + case host + when Regexp then sanitize_regexp(host) + when String then sanitize_string(host) + else host + end + end + end + + def sanitize_regexp(host) + /\A#{host}\z/ + end + + def sanitize_string(host) + if host.start_with?(".") + /\A(.+\.)?#{Regexp.escape(host[1..-1])}\z/ + else + host + end + end + end + + DEFAULT_RESPONSE_APP = -> env do + request = Request.new(env) + + format = request.xhr? ? "text/plain" : "text/html" + template = DebugView.new(host: request.host) + body = template.render(template: "rescues/blocked_host", layout: "rescues/layout") + + [403, { + "Content-Type" => "#{format}; charset=#{Response.default_charset}", + "Content-Length" => body.bytesize.to_s, + }, [body]] + end + + def initialize(app, hosts, response_app = nil) + @app = app + @permissions = Permissions.new(hosts) + @response_app = response_app || DEFAULT_RESPONSE_APP + end + + def call(env) + return @app.call(env) if @permissions.empty? + + request = Request.new(env) + + if authorized?(request) + mark_as_authorized(request) + @app.call(env) + else + @response_app.call(env) + end + end + + private + + def authorized?(request) + origin_host = request.get_header("HTTP_HOST").to_s.sub(/:\d+\z/, "") + forwarded_host = request.x_forwarded_host.to_s.split(/,\s?/).last.to_s.sub(/:\d+\z/, "") + + @permissions.allows?(origin_host) && + (forwarded_host.blank? || @permissions.allows?(forwarded_host)) + end + + def mark_as_authorized(request) + request.set_header("action_dispatch.authorized_host", request.host) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb new file mode 100644 index 0000000000..2fa78dd385 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb @@ -0,0 +1,7 @@ +
+

Blocked host: <%= @host %>

+
+
+

To allow requests to <%= @host %>, add the following configuration:

+ 
Rails.application.config.hosts << "<%= @host %>"
+
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb new file mode 100644 index 0000000000..4e2d1d0b08 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb @@ -0,0 +1,5 @@ +Blocked host: <%= @host %> + +To allow requests to <%= @host %>, add the following configuration: + + Rails.application.config.hosts << "<%= @host %>" diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb new file mode 100644 index 0000000000..dcb59ddb94 --- /dev/null +++ b/actionpack/test/dispatch/host_authorization_test.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class HostAuthorizationTest < ActionDispatch::IntegrationTest + App = -> env { [200, {}, %w(Success)] } + + test "blocks requests to unallowed host" do + @app = ActionDispatch::HostAuthorization.new(App, %w(only.com)) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "passes all requests to if the whitelist is empty" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "passes requests to allowed host" do + @app = ActionDispatch::HostAuthorization.new(App, %w(www.example.com)) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "the whitelist could be a single element" do + @app = ActionDispatch::HostAuthorization.new(App, "www.example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "passes requests to allowed hosts with domain name notation" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "does not allow domain name notation in the HOST header itself" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/", env: { + "HOST" => ".example.com", + } + + assert_response :forbidden + assert_match "Blocked host: .example.com", response.body + end + + test "checks for requests with #=== to support wider range of host checks" do + @app = ActionDispatch::HostAuthorization.new(App, [-> input { input == "www.example.com" }]) + + get "/" + + assert_response :ok + assert_equal "Success", body + end + + test "mark the host when authorized" do + @app = ActionDispatch::HostAuthorization.new(App, ".example.com") + + get "/" + + assert_equal "www.example.com", request.get_header("action_dispatch.authorized_host") + end + + test "sanitizes regular expressions to prevent accidental matches" do + @app = ActionDispatch::HostAuthorization.new(App, [/w.example.co/]) + + get "/" + + assert_response :forbidden + assert_match "Blocked host: www.example.com", response.body + end + + test "blocks requests to unallowed host supporting custom responses" do + @app = ActionDispatch::HostAuthorization.new(App, ["w.example.co"], -> env do + [401, {}, %w(Custom)] + end) + + get "/" + + assert_response :unauthorized + assert_equal "Custom", body + end + + test "blocks requests with spoofed X-FORWARDED-HOST" do + @app = ActionDispatch::HostAuthorization.new(App, [IPAddr.new("127.0.0.1")]) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: 127.0.0.1", response.body + end + + test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do + @app = ActionDispatch::HostAuthorization.new(App, nil) + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "127.0.0.1", + "HOST" => "www.example.com", + } + + assert_response :ok + assert_equal "Success", body + end + + test "detects localhost domain spoofing" do + @app = ActionDispatch::HostAuthorization.new(App, "localhost") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "localhost", + "HOST" => "www.example.com", + } + + assert_response :forbidden + assert_match "Blocked host: localhost", response.body + end + + test "forwarded hosts should be permitted" do + @app = ActionDispatch::HostAuthorization.new(App, "domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :forbidden + assert_match "Blocked host: sub.domain.com", response.body + end + + test "forwarded hosts are allowed when permitted" do + @app = ActionDispatch::HostAuthorization.new(App, ".domain.com") + + get "/", env: { + "HTTP_X_FORWARDED_HOST" => "sub.domain.com", + "HOST" => "domain.com", + } + + assert_response :ok + assert_equal "Success", body + end +end diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index ff6a00f82b..59024b13ef 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,38 @@ +* Introduce guard against DNS rebinding attacks + + The `ActionDispatch::HostAuthorization` is a new middleware that prevent + against DNS rebinding and other `Host` header attacks. It is included in + the development environment by default with the following configuration: + + Rails.application.config.hosts = [ + IPAddr.new("0.0.0.0/0"), # All IPv4 addresses. + IPAddr.new("::/0"), # All IPv6 addresses. + "localhost" # The localhost reserved domain. + ] + + In other environments `Rails.application.config.hosts` is empty and no + `Host` header checks will be done. If you want to guard against header + attacks on production, you have to manually whitelist the allowed hosts + with: + + Rails.application.config.hosts << "product.com" + + The host of a request is checked against the `hosts` entries with the case + operator (`#===`), which lets `hosts` support entries of type `RegExp`, + `Proc` and `IPAddr` to name a few. Here is an example with a regexp. + + # Allow requests from subdomains like `www.product.com` and + # `beta1.product.com`. + Rails.application.config.hosts << /.*\.product\.com/ + + A special case is supported that allows you to whitelist all sub-domains: + + # Allow requests from subdomains like `www.product.com` and + # `beta1.product.com`. + Rails.application.config.hosts << ".product.com" + + *Genadi Samokovarov* + * Remove redundant suffixes on generated helpers. *Gannon McGibbon* diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f30e5d2a0f..22a82c051d 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "ipaddr" require "active_support/core_ext/kernel/reporting" require "active_support/file_update_checker" require "rails/engine/configuration" @@ -11,7 +12,7 @@ module Rails attr_accessor :allow_concurrency, :asset_host, :autoflush_log, :cache_classes, :cache_store, :consider_all_requests_local, :console, :eager_load, :exceptions_app, :file_watcher, :filter_parameters, - :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags, + :force_ssl, :helpers_paths, :hosts, :logger, :log_formatter, :log_tags, :railties_order, :relative_url_root, :secret_key_base, :secret_token, :ssl_options, :public_file_server, :session_options, :time_zone, :reload_classes_only_on_change, @@ -29,6 +30,7 @@ module Rails @filter_parameters = [] @filter_redirect = [] @helpers_paths = [] + @hosts = Array(([IPAddr.new("0.0.0.0/0"), IPAddr.new("::/0"), "localhost"] if Rails.env.development?)) @public_file_server = ActiveSupport::OrderedOptions.new @public_file_server.enabled = true @public_file_server.index_name = "index" diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 433a7ab41f..193cc59f3a 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -13,6 +13,8 @@ module Rails def build_stack ActionDispatch::MiddlewareStack.new do |middleware| + middleware.use ::ActionDispatch::HostAuthorization, config.hosts, config.action_dispatch.hosts_response_app + if config.force_ssl middleware.use ::ActionDispatch::SSL, config.ssl_options end diff --git a/railties/lib/rails/info_controller.rb b/railties/lib/rails/info_controller.rb index 50fe176946..14459623ac 100644 --- a/railties/lib/rails/info_controller.rb +++ b/railties/lib/rails/info_controller.rb @@ -4,7 +4,7 @@ require "rails/application_controller" require "action_dispatch/routing/inspector" class Rails::InfoController < Rails::ApplicationController # :nodoc: - prepend_view_path ActionDispatch::DebugExceptions::RESCUES_TEMPLATE_PATH + prepend_view_path ActionDispatch::DebugView::RESCUES_TEMPLATE_PATH layout -> { request.xhr? ? false : "application" } before_action :require_local! diff --git a/railties/lib/rails/mailers_controller.rb b/railties/lib/rails/mailers_controller.rb index e2d36d7654..95dae3ec2d 100644 --- a/railties/lib/rails/mailers_controller.rb +++ b/railties/lib/rails/mailers_controller.rb @@ -3,7 +3,7 @@ require "rails/application_controller" class Rails::MailersController < Rails::ApplicationController # :nodoc: - prepend_view_path ActionDispatch::DebugExceptions::RESCUES_TEMPLATE_PATH + prepend_view_path ActionDispatch::DebugView::RESCUES_TEMPLATE_PATH before_action :require_local!, unless: :show_previews? before_action :find_preview, :set_locale, only: :preview diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 631f5bac7f..a6396d3509 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -26,6 +26,7 @@ module ApplicationTests assert_equal [ "Webpacker::DevServerProxy", + "ActionDispatch::HostAuthorization", "Rack::Sendfile", "ActionDispatch::Static", "ActionDispatch::Executor", @@ -58,6 +59,7 @@ module ApplicationTests assert_equal [ "Webpacker::DevServerProxy", + "ActionDispatch::HostAuthorization", "Rack::Sendfile", "ActionDispatch::Static", "ActionDispatch::Executor", @@ -140,7 +142,7 @@ module ApplicationTests add_to_config "config.ssl_options = { redirect: { host: 'example.com' } }" boot! - assert_equal [{ redirect: { host: "example.com" } }], Rails.application.middleware[1].args + assert_equal [{ redirect: { host: "example.com" } }], Rails.application.middleware[2].args end test "removing Active Record omits its middleware" do @@ -224,7 +226,7 @@ module ApplicationTests test "insert middleware after" do add_to_config "config.middleware.insert_after Rack::Sendfile, Rack::Config" boot! - assert_equal "Rack::Config", middleware.third + assert_equal "Rack::Config", middleware.fourth end test "unshift middleware" do @@ -236,19 +238,19 @@ module ApplicationTests test "Rails.cache does not respond to middleware" do add_to_config "config.cache_store = :memory_store" boot! - assert_equal "Rack::Runtime", middleware.fifth + assert_equal "Rack::Runtime", middleware[5] end test "Rails.cache does respond to middleware" do boot! - assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware.fifth - assert_equal "Rack::Runtime", middleware[5] + assert_equal "ActiveSupport::Cache::Strategy::LocalCache", middleware[5] + assert_equal "Rack::Runtime", middleware[6] end test "insert middleware before" do add_to_config "config.middleware.insert_before Rack::Sendfile, Rack::Config" boot! - assert_equal "Rack::Config", middleware.second + assert_equal "Rack::Config", middleware.third end test "can't change middleware after it's built" do diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index d4eed69a87..39c936428f 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -197,6 +197,7 @@ module TestHelpers end add_to_config <<-RUBY + config.hosts << proc { true } config.eager_load = false config.session_store :cookie_store, key: "_myapp_session" config.active_support.deprecation = :log @@ -220,6 +221,7 @@ module TestHelpers @app = Class.new(Rails::Application) do def self.name; "RailtiesTestApp"; end end + @app.config.hosts << proc { true } @app.config.eager_load = false @app.config.session_store :cookie_store, key: "_myapp_session" @app.config.active_support.deprecation = :log -- cgit v1.2.3