Merge pull request #48941 from skipkayhil/hm-show-correct-blocked-hosts

Fix host display when X_FORWARDED_HOST authorized

actionpack/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix `HostAuthorization` potentially displaying the value of the
+    X_FORWARDED_HOST header when the HTTP_HOST header is being blocked.
+
+    *Hartley McGuire*, *Daniel Schlosser*
+
 *   Rename `fixture_file_upload` method to `file_fixture_upload`
 
     Declare an alias to preserve the backwards compatibility of `fixture_file_upload`

actionpack/lib/action_dispatch/middleware/host_authorization.rb

@@ -98,7 +98,7 @@ def call(env)
         def response_body(request)
           return "" unless request.get_header("action_dispatch.show_detailed_exceptions")
 
-          template = DebugView.new(host: request.host)
+          template = DebugView.new(hosts: request.env["action_dispatch.blocked_hosts"])
           template.render(template: "rescues/blocked_host", layout: "rescues/layout")
         end
 
@@ -114,7 +114,7 @@ def log_error(request)
 
           return unless logger
 
-          logger.error("[#{self.class.name}] Blocked host: #{request.host}")
+          logger.error("[#{self.class.name}] Blocked hosts: #{request.env["action_dispatch.blocked_hosts"].join(", ")}")
         end
 
         def available_logger(request)
@@ -134,21 +134,28 @@ def call(env)
       return @app.call(env) if @permissions.empty?
 
       request = Request.new(env)
+      hosts = blocked_hosts(request)
 
-      if authorized?(request) || excluded?(request)
+      if hosts.empty? || excluded?(request)
         mark_as_authorized(request)
         @app.call(env)
       else
+        env["action_dispatch.blocked_hosts"] = hosts
         @response_app.call(env)
       end
     end
 
     private
-      def authorized?(request)
+      def blocked_hosts(request)
+        hosts = []
+
         origin_host = request.get_header("HTTP_HOST")
+        hosts << origin_host unless @permissions.allows?(origin_host)
+
         forwarded_host = request.x_forwarded_host&.split(/,\s?/)&.last
+        hosts << forwarded_host unless forwarded_host.blank? || @permissions.allows?(forwarded_host)
 
-        @permissions.allows?(origin_host) && (forwarded_host.blank? || @permissions.allows?(forwarded_host))
+        hosts
       end
 
       def excluded?(request)

actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.html.erb

@@ -1,8 +1,12 @@
 <header>
-  <h1>Blocked host: <%= @host %></h1>
+  <h1>Blocked hosts: <%= @hosts.join(", ") %></h1>
 </header>
 <main role="main" id="container">
-  <h2>To allow requests to <%= @host %> make sure it is a valid hostname (containing only numbers, letters, dashes and dots), then add the following to your environment configuration:</h2>
-  <pre>config.hosts &lt;&lt; "<%= @host %>"</pre>
+  <h2>To allow requests to these hosts, make sure they are valid hostnames (containing only numbers, letters, dashes and dots), then add the following to your environment configuration:</h2>
+  <pre>
+  <% @hosts.each do |host| %>
+    config.hosts &lt;&lt; "<%= host %>"
+  <% end %>
+  </pre>
   <p>For more details view: <a href="https://guides.rubyonrails.org/configuring.html#actiondispatch-hostauthorization">the Host Authorization guide</a></p>
 </main>

actionpack/lib/action_dispatch/middleware/templates/rescues/blocked_host.text.erb

@@ -1,7 +1,9 @@
-Blocked host: <%= @host %>
+Blocked hosts: <%= @hosts.join(", ") %>
 
-To allow requests to <%= @host %> make sure it is a valid hostname (containing only numbers, letters, dashes and dots), then add the following to your environment configuration:
+To allow requests to these hosts, make sure they are valid hostnames (containing only numbers, letters, dashes and dots), then add the following to your environment configuration:
 
-  config.hosts << "<%= @host %>"
+<% @hosts.each do |host| %>
+  config.hosts << "<%= host %>"
+<% end %>
 
 For more details on host authorization view: https://guides.rubyonrails.org/configuring.html#actiondispatch-hostauthorization

actionpack/test/dispatch/host_authorization_test.rb

@@ -21,7 +21,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: { "action_dispatch.show_detailed_exceptions" => true }
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.com", response.body
+    assert_match "Blocked hosts: www.example.com", response.body
   end
 
   test "allows all requests if hosts is empty" do
@@ -93,7 +93,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.local", response.body
+    assert_match "Blocked hosts: www.example.local", response.body
   end
 
   test "passes requests to allowed hosts with domain name notation" do
@@ -114,7 +114,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: .example.com", response.body
+    assert_match "Blocked hosts: .example.com", response.body
   end
 
   test "checks for requests with #=== to support wider range of host checks" do
@@ -140,7 +140,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: { "action_dispatch.show_detailed_exceptions" => true }
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.com", response.body
+    assert_match "Blocked hosts: www.example.com", response.body
   end
 
   test "blocks requests to unallowed host supporting custom responses" do
@@ -284,7 +284,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: 127.0.0.1", response.body
+    assert_match "Blocked hosts: www.example.com", response.body
   end
 
   test "blocks requests with spoofed relative X-FORWARDED-HOST" do
@@ -297,7 +297,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: //randomhost.com", response.body
+    assert_match "Blocked hosts: //randomhost.com", response.body
   end
 
   test "forwarded secondary hosts are allowed when permitted" do
@@ -322,7 +322,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: evil.com", response.body
+    assert_match "Blocked hosts: evil.com", response.body
   end
 
   test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do
@@ -347,7 +347,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: localhost", response.body
+    assert_match "Blocked hosts: www.example.com", response.body
   end
 
   test "forwarded hosts should be permitted" do
@@ -360,7 +360,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: sub.domain.com", response.body
+    assert_match "Blocked hosts: sub.domain.com", response.body
   end
 
   test "sub-sub domains should not be permitted" do
@@ -372,7 +372,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: secondary.sub.domain.com", response.body
+    assert_match "Blocked hosts: secondary.sub.domain.com", response.body
   end
 
   test "forwarded hosts are allowed when permitted" do
@@ -420,7 +420,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
       }
 
       assert_response :forbidden
-      assert_match "Blocked host: #{host}", response.body
+      assert_match "Blocked hosts: #{host}", response.body
     end
   end
 
@@ -439,7 +439,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/foo", env: { "action_dispatch.show_detailed_exceptions" => true }
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.com", response.body
+    assert_match "Blocked hosts: www.example.com", response.body
   end
 
   test "blocks requests with invalid hostnames" do
@@ -451,7 +451,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: attacker.com#x.example.com", response.body
+    assert_match "Blocked hosts: attacker.com#x.example.com", response.body
   end
 
   test "blocks requests to similar host" do
@@ -463,7 +463,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     }
 
     assert_response :forbidden
-    assert_match "Blocked host: sub-example.com", response.body
+    assert_match "Blocked hosts: sub-example.com", response.body
   end
 
   test "uses logger from the env" do
@@ -473,7 +473,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     get "/", env: { "action_dispatch.logger" => Logger.new(output) }
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.com", output.rewind && output.read
+    assert_match "Blocked hosts: www.example.com", output.rewind && output.read
   end
 
   test "uses ActionView::Base logger when no logger in the env" do
@@ -489,7 +489,7 @@ class HostAuthorizationTest < ActionDispatch::IntegrationTest
     end
 
     assert_response :forbidden
-    assert_match "Blocked host: www.example.com", output.rewind && output.read
+    assert_match "Blocked hosts: www.example.com", output.rewind && output.read
   end
 
   private