Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support RFC 7239: HTTP Forwarded header #1423

Closed
wants to merge 1 commit into from
Closed

Support RFC 7239: HTTP Forwarded header #1423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 14 additions & 13 deletions lib/rack/common_logger.rb
View file
Open in desktop
Expand Up @@ -32,30 +32,31 @@ def initialize(app, logger = nil)

def call(env)
began_at = Utils.clock_time
status, header, body = @app.call(env)
header = Utils::HeaderHash.new(header)
body = BodyProxy.new(body) { log(env, status, header, began_at) }
[status, header, body]
status, headers, body = @app.call(env)
headers = Utils::HeaderHash.new(headers)
body = BodyProxy.new(body) { log(env, status, headers, began_at) }
[status, headers, body]
end

private

def log(env, status, header, began_at)
length = extract_content_length(header)
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[PATH_INFO],
env[QUERY_STRING].empty? ? "" : "?#{env[QUERY_STRING]}",
env[SERVER_PROTOCOL],
request.request_method,
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)
Expand Down
28 changes: 19 additions & 9 deletions lib/rack/request.rb
View file
Open in desktop
Expand Up @@ -134,6 +134,7 @@ module Helpers
# to include the port in a generated URI.
DEFAULT_PORTS = { 'http' => 80, 'https' => 443, 'coffee' => 80 }

HTTP_FORWARDED = 'HTTP_FORWARDED'
HTTP_X_FORWARDED_SCHEME = 'HTTP_X_FORWARDED_SCHEME'
HTTP_X_FORWARDED_PROTO = 'HTTP_X_FORWARDED_PROTO'
HTTP_X_FORWARDED_HOST = 'HTTP_X_FORWARDED_HOST'
Expand Down Expand Up @@ -238,28 +239,26 @@ def xhr?
end

def host_with_port
if forwarded = get_header(HTTP_X_FORWARDED_HOST)
if forwarded = get_http_forwarded(:host)
forwarded.last
elsif forwarded = get_header(HTTP_X_FORWARDED_HOST)
Copy link

@lunich lunich May 27, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible that get_header(HTTP_X_FORWARDED_HOST) == '' in some cases. For instance:

curl http://example.com -H 'X_FORWARDED_HOST;' -H 'HOST: example.com'

or

telnet example.com 80
Trying 127.0.0.1...
Connected to example.com.
Escape character is '^]'.
GET / HTTP/1.0
HOST: example.com
X_FORWARDED_HOST:

forwarded.split(/,\s?/).last
else
get_header(HTTP_HOST) || "#{get_header(SERVER_NAME) || get_header(SERVER_ADDR)}:#{get_header(SERVER_PORT)}"
end
end

def host
# Remove port number.
h = host_with_port
if colon_index = h.index(":")
h[0, colon_index]
else
h
end
strip_port(host_with_port)
end

def port
if port = extract_port(host_with_port)
port.to_i
elsif port = get_header(HTTP_X_FORWARDED_PORT)
port.to_i
elsif get_http_forwarded(:proto)
DEFAULT_PORTS[scheme]
elsif has_header?(HTTP_X_FORWARDED_HOST)
DEFAULT_PORTS[scheme]
elsif has_header?(HTTP_X_FORWARDED_PROTO)
Expand All @@ -279,7 +278,10 @@ def ip

return remote_addrs.first if remote_addrs.any?

forwarded_ips = split_ip_addresses(get_header('HTTP_X_FORWARDED_FOR'))
rfc7239_forwarded_ips = get_http_forwarded(:for) || []
x_forwarded_ips = split_ip_addresses(get_header('HTTP_X_FORWARDED_FOR'))

forwarded_ips = x_forwarded_ips.concat(rfc7239_forwarded_ips)
.map { |ip| strip_port(ip) }

return reject_trusted_ip_addresses(forwarded_ips).last || forwarded_ips.first || get_header("REMOTE_ADDR")
Expand Down Expand Up @@ -482,6 +484,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
Expand Down Expand Up @@ -522,6 +530,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
Expand Down
15 changes: 15 additions & 0 deletions lib/rack/utils.rb
View file
Open in desktop
Expand Up @@ -146,6 +146,21 @@ def q_values(q_value_header)
end
module_function :q_values

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

def best_q_match(q_value_header, available_mimes)
values = q_values(q_value_header)

Expand Down
16 changes: 15 additions & 1 deletion test/spec_common_logger.rb
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@
log.string.must_match(/"GET \/ " 200 #{length} /)
end

it "work with standartd library logger" do
it "work with standard library logger" do
logdev = StringIO.new
log = Logger.new(logdev)
Rack::MockRequest.new(Rack::CommonLogger.new(app, log)).get("/")
Expand All @@ -60,6 +60,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
Expand Down
55 changes: 55 additions & 0 deletions test/spec_request.rb
View file
Open in desktop
Expand Up @@ -122,6 +122,23 @@ class RackRequestTest < Minitest::Spec
Rack::MockRequest.env_for("/", "SERVER_NAME" => "example.org", "SERVER_PORT" => "9292")
req.host.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"
Expand Down Expand Up @@ -185,6 +202,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
Expand All @@ -207,6 +228,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
Expand Down Expand Up @@ -541,6 +566,18 @@ def initialize(*)
request.scheme.must_equal "http"
request.wont_be :ssl?

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'))
request.scheme.must_equal "https"
request.must_be :ssl?
Expand Down Expand Up @@ -1193,6 +1230,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'
Expand Down Expand Up @@ -1227,6 +1279,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'

Expand Down
37 changes: 37 additions & 0 deletions test/spec_utils.rb
View file
Open in desktop
Expand Up @@ -378,6 +378,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"

Expand Down