Allow configuring priorities for Forwarded and X-Forwarded-*

The Request.forwarded_priority accessor sets the priority. Default to considering Forwarded first, since it is now the official standard. Also allow configuring whether X-Forwarded-Proto or X-Forwarded-Scheme has priority, using the Request.x_forwarded_proto_priority accessor. Allowing configurable priorities for these headers is necessary, because which headers should be checked depends on the environment the application runs in. Make Request#forwarded_authority use the last forwarded authority instead of the first forwarded authority, since earlier forwarded authorities can be forged by the client. Fixes #1809 Fixes #1829 Implements #1423 Implements #1832
rack · Apr 5, 2022 · b87d182 · b87d182
1 parent 58eed30
commit b87d182
Show file tree

Hide file tree

Showing 3 changed files with 289 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,7 +29,7 @@ All notable changes to this project will be documented in this file. For info on
 - `Rack::Utils#set_cookie_header` now supports `escape_key: false` to avoid key escaping.  ([@jeremyevans](https://github.com/jeremyevans))
 - `Rack::RewindableInput` supports size. ([@ahorek](https://github.com/ahorek))
 - `Rack::RewindableInput::Middleware` added for making `rack.input` rewindable. ([@jeremyevans](https://github.com/jeremyevans))
-- Rack::Session::Pool now accepts `:allow_fallback` option to disable fallback to public id. ([#1431](https://github.com/rack/rack/issues/1431), [@jeremyevans](https://github.com/jeremyevans))
+- The RFC 7239 Forwarded header is now supported and considered by default when looking for information on forwarding, falling back to the the X-Forwarded-* headers. `Rack::Request.forwarded_priority` accessor has been added for configuring the priority of which header to check.  ([#1423](https://github.com/rack/rack/issues/1423), [@jeremyevans](https://github.com/jeremyevans))
 
 ### Changed
 
@@ -49,6 +49,8 @@ All notable changes to this project will be documented in this file. For info on
 - Explicitly deprecate `Rack::File` which was an alias for `Rack::Files`. ([#1811](https://github.com/rack/rack/pull/1720), [@ioquatix](https://github.com/ioquatix)).
 - Moved `Rack::Session` into [separate gem](https://github.com/rack/rack-session). ([#1805](https://github.com/rack/rack/pull/1805), [@ioquatix](https://github.com/ioquatix))
 - rackup -D option to daemonizes no longer changes the working directory to the root. ([#1813](https://github.com/rack/rack/pull/1813), [@jeremyevans](https://github.com/jeremyevans))
+- The X-Forwarded-Proto header is now considered before the X-Forwarded-Scheme header for determining the forwarded protocol. `Rack::Request.x_forwarded_proto_priority` accessor has been added for configuring the priority of which header to check.  ([#1809](https://github.com/rack/rack/issues/1809), [@jeremyevans](https://github.com/jeremyevans))
+- `Rack::Request.forwarded_authority` (and methods that call it, such as `host`) now returns the last authority in the forwarded header, instead of the first, as earlier forwarded authorities can be forged by clients. This restores the Rack 2.1 behavior. ([#1829](https://github.com/rack/rack/issues/1809), [@jeremyevans](https://github.com/jeremyevans))
 
 ### Fixed
 

diff --git a/lib/rack/request.rb b/lib/rack/request.rb
@@ -16,8 +16,33 @@ module Rack
   class Request
     class << self
       attr_accessor :ip_filter
+
+      # The priority when checking forwarded headers. The default
+      # is <tt>[:forwarded, :x_forwarded]</tt>, which means, check the
+      # +Forwarded+ header first, followed by the appropriate
+      # <tt>X-Forwarded-*</tt> header.  You can revert the priority by
+      # reversing the priority, or remove checking of either
+      # or both headers by removing elements from the array.
+      #
+      # This should be set as appropriate in your environment
+      # based on what reverse proxies are in use.  If you are not
+      # using reverse proxies, you should probably use an empty
+      # array.
+      attr_accessor :forwarded_priority
+
+      # The priority when checking either the <tt>X-Forwarded-Proto</tt>
+      # or <tt>X-Forwarded-Scheme</tt> header for the forwarded protocol.
+      # The default is <tt>[:proto, :scheme]</tt>, to try the
+      # <tt>X-Forwarded-Proto</tt> header before the
+      # <tt>X-Forwarded-Scheme</tt> header.  Rack 2 had behavior
+      # similar to <tt>[:scheme, :proto]</tt>.  You can remove either or
+      # both of the entries in array to ignore that respective header.
+      attr_accessor :x_forwarded_proto_priority
     end
 
+    @forwarded_priority = [:forwarded, :x_forwarded]
+    @x_forwarded_proto_priority = [:proto, :scheme]
+
     valid_ipv4_octet = /\.(25[0-5]|2[0-4][0-9]|[01]?[0-9]?[0-9])/
 
     trusted_proxies = Regexp.union(
@@ -342,37 +367,60 @@ def port
       end
 
       def forwarded_for
-        if forwarded_for = get_http_forwarded(:for)
-          forwarded_for.map! do |authority|
-            split_authority(authority)[1]
+        forwarded_priority.each do |type|
+          case type
+          when :forwarded
+            if forwarded_for = get_http_forwarded(:for)
+              return(forwarded_for.map! do |authority|
+                split_authority(authority)[1]
+              end)
+            end
+          when :x_forwarded
+            if value = get_header(HTTP_X_FORWARDED_FOR)
+              return(split_header(value).map do |authority|
+                split_authority(wrap_ipv6(authority))[1]
+              end)
+            end
           end
         end
 
-        if value = get_header(HTTP_X_FORWARDED_FOR)
-          x_forwarded_for = split_header(value).map do |authority|
-            split_authority(wrap_ipv6(authority))[1]
-          end
-        end
-
-        forwarded_for || x_forwarded_for
+        nil
       end
 
       def forwarded_port
-        if forwarded = get_http_forwarded(:for)
-          forwarded.map do |authority|
-            split_authority(authority)[2]
-          end.compact!
-        elsif value = get_header(HTTP_X_FORWARDED_PORT)
-          split_header(value).map(&:to_i)
+        forwarded_priority.each do |type|
+          case type
+          when :forwarded
+            if forwarded = get_http_forwarded(:for)
+              return(forwarded.map do |authority|
+                split_authority(authority)[2]
+              end.compact)
+            end
+          when :x_forwarded
+            if value = get_header(HTTP_X_FORWARDED_PORT)
+              return split_header(value).map(&:to_i)
+            end
+          end
         end
+
+        nil
       end
 
       def forwarded_authority
-        if forwarded = get_http_forwarded(:host)
-          forwarded.last
-        elsif value = get_header(HTTP_X_FORWARDED_HOST)
-          wrap_ipv6(split_header(value).last)
+        forwarded_priority.each do |type|
+          case type
+          when :forwarded
+            if forwarded = get_http_forwarded(:host)
+              return forwarded.last
+            end
+          when :x_forwarded
+            if value = get_header(HTTP_X_FORWARDED_HOST)
+              return wrap_ipv6(split_header(value).last)
+            end
+          end
         end
+
+        nil
       end
 
       def ssl?
@@ -607,8 +655,7 @@ def parse_http_accept_header(header)
 
       # Get an array of values set in the RFC 7239 `Forwarded` request header.
       def get_http_forwarded(token)
-        values = Utils.forwarded_values(get_header(HTTP_FORWARDED))
-        values[token] if values
+        Utils.forwarded_values(get_header(HTTP_FORWARDED))&.[](token)
       end
 
       def query_parser
@@ -676,11 +723,33 @@ def reject_trusted_ip_addresses(ip_addresses)
         ip_addresses.reject { |ip| trusted_proxy?(ip) }
       end
 
+      FORWARDED_SCHEME_HEADERS = {
+        proto: HTTP_X_FORWARDED_PROTO,
+        scheme: HTTP_X_FORWARDED_SCHEME
+      }.freeze
+      private_constant :FORWARDED_SCHEME_HEADERS
       def forwarded_scheme
-        forwarded_proto = get_http_forwarded(:proto)
-        (forwarded_proto && allowed_scheme(forwarded_proto.first)) ||
-        allowed_scheme(get_header(HTTP_X_FORWARDED_SCHEME)) ||
-        allowed_scheme(extract_proto_header(get_header(HTTP_X_FORWARDED_PROTO)))
+        forwarded_priority.each do |type|
+          case type
+          when :forwarded
+            if (forwarded_proto = get_http_forwarded(:proto)) &&
+               (scheme = allowed_scheme(forwarded_proto.last))
+              return scheme
+            end
+          when :x_forwarded
+            x_forwarded_proto_priority.each do |x_type|
+              if header = FORWARDED_SCHEME_HEADERS[x_type]
+                split_header(get_header(header)).reverse_each do |scheme|
+                  if allowed_scheme(scheme)
+                    return scheme
+                  end
+                end
+              end
+            end
+          end
+        end
+
+        nil
       end
 
       def allowed_scheme(header)
@@ -696,6 +765,14 @@ def extract_proto_header(header)
           end
         end
       end
+
+      def forwarded_priority
+        Request.forwarded_priority
+      end
+
+      def x_forwarded_proto_priority
+        Request.x_forwarded_proto_priority
+      end
     end
 
     include Env