From bbc600104d6818cdb759a04b9461fa760d2e51af Mon Sep 17 00:00:00 2001 From: fatkodima Date: Fri, 29 Nov 2019 22:32:46 +0200 Subject: [PATCH] Support RFC 7239: HTTP Forwarded header Co-authored-by: Matt Bostock Co-authored-by: Jeremy Evans --- lib/rack/common_logger.rb | 22 ++++++++------- lib/rack/request.rb | 36 ++++++++++++++++++++---- lib/rack/utils.rb | 15 ++++++++++ test/spec_common_logger.rb | 14 ++++++++++ test/spec_request.rb | 56 +++++++++++++++++++++++++++++++++++++- test/spec_utils.rb | 37 +++++++++++++++++++++++++ 6 files changed, 163 insertions(+), 17 deletions(-) diff --git a/lib/rack/common_logger.rb b/lib/rack/common_logger.rb index 6bb6ba2fe..d833e2e11 100644 --- a/lib/rack/common_logger.rb +++ b/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) diff --git a/lib/rack/request.rb b/lib/rack/request.rb index e633540bd..abb41eebb 100644 --- a/lib/rack/request.rb +++ b/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 diff --git a/lib/rack/utils.rb b/lib/rack/utils.rb index 5292bc892..cd31a9d5e 100644 --- a/lib/rack/utils.rb +++ b/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 diff --git a/test/spec_common_logger.rb b/test/spec_common_logger.rb index 60db77040..3d562ae55 100644 --- a/test/spec_common_logger.rb +++ b/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 diff --git a/test/spec_request.rb b/test/spec_request.rb index 92316f383..266b52a63 100644 --- a/test/spec_request.rb +++ b/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' diff --git a/test/spec_utils.rb b/test/spec_utils.rb index d95d03c9b..116b19462 100644 --- a/test/spec_utils.rb +++ b/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"