Support RFC 7239: HTTP Forwarded header

Co-authored-by: Matt Bostock <matt@mattbostock.com>
Co-authored-by: Jeremy Evans <code@jeremyevans.net>

lib/rack/common_logger.rb

@@ -3,6 +3,7 @@
 require_relative 'constants'
 require_relative 'utils'
 require_relative 'body_proxy'
+require_relative 'request'
 
 module Rack
   # Rack::CommonLogger forwards every request to the given +app+, and
@@ -47,23 +48,24 @@ def call(env)
     private
 
     # Log the request to the configured logger.
-    def log(env, status, headers, began_at)
-      length = extract_content_length(headers)
+    def log(env, status, response_headers, began_at)
+      request = Rack::Request.new(env)
+      length = extract_content_length(response_headers)
 
       msg = FORMAT % [
-        env['HTTP_X_FORWARDED_FOR'] || env["REMOTE_ADDR"] || "-",
-        env["REMOTE_USER"] || "-",
+        request.ip || "-",
+        request.get_header("REMOTE_USER") || "-",
         Time.now.strftime("%d/%b/%Y:%H:%M:%S %z"),
-        env[REQUEST_METHOD],
-        env[SCRIPT_NAME],
-        env[PATH_INFO],
-        env[QUERY_STRING].empty? ? "" : "?#{env[QUERY_STRING]}",
-        env[SERVER_PROTOCOL],
+        request.request_method,
+        request.script_name,
+        request.path_info,
+        request.query_string.empty? ? "" : "?#{request.query_string}",
+        request.get_header(SERVER_PROTOCOL),
         status.to_s[0..3],
         length,
         Utils.clock_time - began_at ]
 
-      logger = @logger || env[RACK_ERRORS]
+      logger = @logger || request.get_header(RACK_ERRORS)
       # Standard library logger doesn't support write but it supports << which actually
       # calls to write on the log device without formatting
       if logger.respond_to?(:write)

lib/rack/request.rb

@@ -152,6 +152,8 @@ module Helpers
       # The contents of the host/:authority header sent to the proxy.
       HTTP_X_FORWARDED_HOST = 'HTTP_X_FORWARDED_HOST'
 
+      HTTP_FORWARDED          = 'HTTP_FORWARDED'
+
       # The value of the scheme sent to the proxy.
       HTTP_X_FORWARDED_SCHEME = 'HTTP_X_FORWARDED_SCHEME'
 
@@ -331,7 +333,7 @@ def port
         end
 
         if forwarded_port = self.forwarded_port
-          return forwarded_port.first
+          return forwarded_port.last
         end
 
         if scheme = self.scheme
@@ -344,22 +346,36 @@ def port
       end
 
       def forwarded_for
+        if forwarded_for = get_http_forwarded(:for)
+          forwarded_for.map! do |authority|
+            split_authority(authority)[1]
+          end
+        end
+
         if value = get_header(HTTP_X_FORWARDED_FOR)
-          split_header(value).map do |authority|
+          x_forwarded_for = split_header(value).map do |authority|
             split_authority(wrap_ipv6(authority))[1]
           end
         end
+
+        forwarded_for || x_forwarded_for
       end
 
       def forwarded_port
-        if value = get_header(HTTP_X_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)
         end
       end
 
       def forwarded_authority
-        if value = get_header(HTTP_X_FORWARDED_HOST)
-          wrap_ipv6(split_header(value).first)
+        if forwarded = get_http_forwarded(:host)
+          forwarded.last
+        elsif value = get_header(HTTP_X_FORWARDED_HOST)
+          wrap_ipv6(split_header(value).last)
         end
       end
 
@@ -372,7 +388,7 @@ def ip
         external_addresses = reject_trusted_ip_addresses(remote_addresses)
 
         unless external_addresses.empty?
-          return external_addresses.first
+          return external_addresses.last
         end
 
         if forwarded_for = self.forwarded_for
@@ -593,6 +609,12 @@ def parse_http_accept_header(header)
         end
       end
 
+      # 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
+      end
+
       def query_parser
         Utils.default_query_parser
       end
@@ -639,6 +661,8 @@ def reject_trusted_ip_addresses(ip_addresses)
       end
 
       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)))
       end

lib/rack/utils.rb

@@ -140,6 +140,21 @@ def q_values(q_value_header)
       end
     end
 
+    FORWARDED_PAIR_REGEX = /\A\s*(by|for|host|proto)\s*=\s*"?([^"]+)"?\s*\Z/i
+
+    def forwarded_values(forwarded_header)
+      return nil unless forwarded_header
+      forwarded_header = forwarded_header.to_s.gsub("\n", ";")
+
+      forwarded_header.split(/\s*;\s*/).each_with_object({}) do |field, values|
+        field.split(/\s*,\s*/).each do |pair|
+          return nil unless pair =~ FORWARDED_PAIR_REGEX
+          (values[$1.downcase.to_sym] ||= []) << $2
+        end
+      end
+    end
+    module_function :forwarded_values
+
     # Return best accept value to use, based on the algorithm
     # in RFC 2616 Section 14.  If there are multiple best
     # matches (same specificity and quality), the value returned

test/spec_common_logger.rb

@@ -62,6 +62,20 @@
     res.errors.must_match(/"GET \/ " 200 - /)
   end
 
+  it "log - records host from X-Forwarded-For header" do
+    res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/", 'HTTP_X_FORWARDED_FOR' => '203.0.113.0')
+
+    res.errors.wont_be :empty?
+    res.errors.must_match(/203\.0\.113\.0 - /)
+  end
+
+  it "log - records host from RFC 7239 forwarded for header" do
+    res = Rack::MockRequest.new(Rack::CommonLogger.new(app)).get("/", 'HTTP_FORWARDED' => 'for=203.0.113.0')
+
+    res.errors.wont_be :empty?
+    res.errors.must_match(/203\.0\.113\.0 - /)
+  end
+
   def with_mock_time(t = 0)
     mc = class << Time; self; end
     mc.send :alias_method, :old_now, :now

test/spec_request.rb

@@ -162,6 +162,23 @@ class RackRequestTest < Minitest::Spec
     req.host.must_equal "example.org"
     req.hostname.must_equal "example.org"
 
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=example.org:9292")
+    req.host.must_equal "example.org"
+
+    # Test obfuscated identifier: https://tools.ietf.org/html/rfc7239#section-6.3
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=ObFuScaTeD")
+    req.host.must_equal "ObFuScaTeD"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_FORWARDED" => "host=example.com; host=example.org:9292")
+    req.host.must_equal "example.org"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org:9292", "HTTP_FORWARDED" => "host=example.com")
+    req.host.must_equal "example.com"
+
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org:9292")
     req.host.must_equal "example.org"
@@ -239,6 +256,10 @@ class RackRequestTest < Minitest::Spec
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost", "HTTP_X_FORWARDED_PROTO" => "https,https", "SERVER_PORT" => "80")
     req.port.must_equal 443
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost", "HTTP_FORWARDED" => "proto=https", "HTTP_X_FORWARDED_PROTO" => "http", "SERVER_PORT" => "9393")
+    req.port.must_equal 443
   end
 
   it "figure out the correct host with port" do
@@ -273,6 +294,10 @@ class RackRequestTest < Minitest::Spec
     req = make_request \
       Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org", "SERVER_PORT" => "9393")
     req.host_with_port.must_equal "example.org"
+
+    req = make_request \
+      Rack::MockRequest.env_for("/", "HTTP_HOST" => "localhost:81", "HTTP_X_FORWARDED_HOST" => "example.org", "HTTP_FORWARDED" => "host=example.com:9292", "SERVER_PORT" => "9393")
+    req.host_with_port.must_equal "example.com:9292"
   end
 
   it "parse the query string" do
@@ -630,6 +655,17 @@ def initialize(*)
 
     request = make_request(Rack::MockRequest.env_for("/", 'HTTP_X_FORWARDED_PROTO' => 'ws'))
     request.scheme.must_equal "ws"
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=https'))
+    request.scheme.must_equal "https"
+    request.must_be :ssl?
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=https, proto=http'))
+    request.scheme.must_equal "https"
+    request.must_be :ssl?
+
+    request = make_request(Rack::MockRequest.env_for("/", 'HTTP_FORWARDED' => 'proto=http, proto=https'))
+    request.scheme.must_equal "http"
     request.wont_be :ssl?
 
     request = make_request(Rack::MockRequest.env_for("/", 'HTTPS' => 'on'))
@@ -1289,7 +1325,7 @@ def ip_app
     res.body.must_equal 'fe80::202:b3ff:fe1e:8329'
 
     res = mock.get '/', 'REMOTE_ADDR' => '1.2.3.4,3.4.5.6'
-    res.body.must_equal '1.2.3.4'
+    res.body.must_equal '3.4.5.6'
 
     res = mock.get '/', 'REMOTE_ADDR' => '127.0.0.1'
     res.body.must_equal '127.0.0.1'
@@ -1301,6 +1337,21 @@ def ip_app
   it 'deals with proxies' do
     mock = Rack::MockRequest.new(Rack::Lint.new(ip_app))
 
+    res = mock.get '/',
+      'REMOTE_ADDR' => '1.2.3.4',
+      'HTTP_FORWARDED' => 'for=3.4.5.6'
+    res.body.must_equal '1.2.3.4'
+
+    res = mock.get '/',
+      'HTTP_X_FORWARDED_FOR' => '3.4.5.6',
+      'HTTP_FORWARDED' => 'for=5.6.7.8'
+    res.body.must_equal '5.6.7.8'
+
+    res = mock.get '/',
+      'HTTP_X_FORWARDED_FOR' => '3.4.5.6',
+      'HTTP_FORWARDED' => 'for=5.6.7.8, for=7.8.9.0'
+    res.body.must_equal '7.8.9.0'
+
     res = mock.get '/',
       'REMOTE_ADDR' => '1.2.3.4',
       'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
@@ -1335,6 +1386,9 @@ def ip_app
     res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '[2001:db8:cafe::17]:47011'
     res.body.must_equal '2001:db8:cafe::17'
 
+    res = mock.get '/', 'HTTP_FORWARDED' => 'for="[2001:db8:cafe::17]:47011"'
+    res.body.must_equal '2001:db8:cafe::17'
+
     res = mock.get '/', 'HTTP_X_FORWARDED_FOR' => '1.2.3.4, [2001:db8:cafe::17]:47011'
     res.body.must_equal '2001:db8:cafe::17'
 

test/spec_utils.rb

@@ -417,6 +417,43 @@ def initialize(*)
     ]
   end
 
+  it "parses RFC 7239 Forwarded header" do
+    Rack::Utils.forwarded_values('for=3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values(';;;for=3.4.5.6,,').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for =  3.4.5.6').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for="3.4.5.6"').must_equal({
+      for: [ '3.4.5.6' ],
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6;proto=https').must_equal({
+      for: [ '3.4.5.6' ],
+      proto: [ 'https' ]
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6; proto=http, proto=https').must_equal({
+      for: [ '3.4.5.6' ],
+      proto: [ 'http', 'https' ]
+    })
+
+    Rack::Utils.forwarded_values('for=3.4.5.6; proto=http, proto=https; for=1.2.3.4').must_equal({
+      for: [ '3.4.5.6', '1.2.3.4' ],
+      proto: [ 'http', 'https' ]
+    })
+  end
+
   it "select best quality match" do
     Rack::Utils.best_q_match("text/html", %w[text/html]).must_equal "text/html"