diff --git a/lib/falcon/middleware/proxy.rb b/lib/falcon/middleware/proxy.rb index 0a4c1e9..cc91b76 100644 --- a/lib/falcon/middleware/proxy.rb +++ b/lib/falcon/middleware/proxy.rb @@ -38,6 +38,34 @@ class Proxy < Protocol::HTTP::Middleware VIA = "via" CONNECTION = "connection" + # Forwarding headers which carry trust-sensitive information about the + # original client (their address and the request scheme). Because Falcon + # acts as the trust boundary, any client-supplied values are untrustworthy, + # so we strip every inbound forwarding header and author our own below from + # connection-level facts. + # + # We emit both the modern RFC 7239 {FORWARDED} header and the legacy + # `x-forwarded-for` / `x-forwarded-proto` headers, because many downstream + # consumers still read the legacy ones. Notably Rack's + # `Rack::Request#forwarded_for` (used by Rails' `ActionDispatch::RemoteIp`) + # only prefers `Forwarded` and falls back to `X-Forwarded-For`; older Rack + # (< 3) and a lot of application code read `X-Forwarded-For` directly. + # + # We also strip underscore variants of the same header names because Rack + # normalizes both `x-forwarded-for` and `x_forwarded_for` to the same + # `HTTP_X_FORWARDED_FOR` CGI environment key. + FORWARDING_HEADERS = [ + FORWARDED, + X_FORWARDED_FOR, + X_FORWARDED_PROTO, + "x-forwarded-host", + "x-forwarded-port", + "x_forwarded_for", + "x_forwarded_proto", + "x_forwarded_host", + "x_forwarded_port", + ] + # HTTP hop headers which *should* not be passed through the proxy. HOP_HEADERS = [ "connection", @@ -101,10 +129,13 @@ def prepare_headers(headers) end # Prepare the request to be proxied to the specified host. - # In particular, we set appropriate {VIA}, {FORWARDED}, {X_FORWARDED_FOR} and {X_FORWARDED_PROTO} headers. + # + # Falcon acts as the trust boundary, so we strip any client-supplied + # {FORWARDING_HEADERS} and author our own from connection-level facts: the + # RFC 7239 {FORWARDED} header plus the legacy {X_FORWARDED_FOR} / + # {X_FORWARDED_PROTO} headers, along with an appended {VIA} header. This + # prevents a client from spoofing the forwarded address or scheme. def prepare_request(request, host) - forwarded = [] - Console.debug(self) do |buffer| buffer.puts "Request authority: #{request.authority}" buffer.puts "Host authority: #{host.authority}" @@ -115,9 +146,17 @@ def prepare_request(request, host) # The authority of the request must match the authority of the endpoint we are proxying to, otherwise SNI and other things won't work correctly. request.authority = host.authority + # Discard inbound hop-by-hop headers before authoring trusted forwarding + # headers, so client-supplied Connection tokens can't remove headers we add. + self.prepare_headers(request.headers) + # Discard any inbound forwarding headers so a client can't spoof them; we author our own below from connection-level facts. + request.headers.extract(FORWARDING_HEADERS) + + forwarded = [] + if address = request.remote_address request.headers.add(X_FORWARDED_FOR, address.ip_address) - forwarded << "for=#{address.ip_address}" + forwarded << "for=#{forwarded_node(address)}" end if scheme = request.scheme @@ -131,11 +170,21 @@ def prepare_request(request, host) request.headers.add(VIA, "#{request.version} #{self.class}") - self.prepare_headers(request.headers) - return request end + # Format a remote address as an RFC 7239 `for=` node identifier. + # IPv6 addresses must be enclosed in square brackets and quoted. + # @parameter address [Addrinfo] The remote address of the client. + # @returns [String] The node identifier for use in a {FORWARDED} header. + def forwarded_node(address) + if address.ipv6? + "\"[#{address.ip_address}]\"" + else + address.ip_address + end + end + # Proxy the request if the authority matches a specific host. # @parameter request [Protocol::HTTP::Request] def call(request) diff --git a/test/falcon/middleware/proxy.rb b/test/falcon/middleware/proxy.rb index 8ee2131..9e4007a 100644 --- a/test/falcon/middleware/proxy.rb +++ b/test/falcon/middleware/proxy.rb @@ -27,6 +27,8 @@ def proxy_for(**options) let(:headers) {Protocol::HTTP::Headers["accept" => "*/*"]} + let(:host) {proxy_for(authority: "www.google.com", endpoint: Async::HTTP::Endpoint.parse("https://www.google.com"))} + it "removes proxy authorization by default" do headers = Protocol::HTTP::Headers[ "authorization" => "Bearer application", @@ -47,11 +49,106 @@ def proxy_for(**options) expect(response).not.to be(:failure?) + expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=https"] expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"] proxy.close end + it "authors a forwarded header from the connection" do + request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1")) + + proxy.prepare_request(request, host) + + expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=https"] + expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"] + expect(request.headers["x-forwarded-proto"]).to be == ["https"] + expect(request.headers["via"]).not.to be_nil + end + + it "strips client-supplied forwarding headers so they can't be spoofed" do + headers = Protocol::HTTP::Headers[ + "x-forwarded-for" => "1.2.3.4", + "x-forwarded-proto" => "https", + "x-forwarded-host" => "evil.example.com", + "x-forwarded-port" => "8443", + "forwarded" => "for=9.9.9.9;proto=https", + ] + request = Protocol::HTTP::Request.new("http", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1")) + + proxy.prepare_request(request, host) + + # The spoofed values are stripped and replaced with Falcon's own. + expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"] + expect(request.headers["x-forwarded-proto"]).to be == ["http"] + expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=http"] + + # `x-forwarded-host` and `x-forwarded-port` are stripped and not re-authored, so they can't be spoofed. + expect(request.headers["x-forwarded-host"]).to be_nil + expect(request.headers["x-forwarded-port"]).to be_nil + end + + it "strips underscore forwarding headers that collide with rack environment keys" do + headers = Protocol::HTTP::Headers[ + "x_forwarded_for" => "1.2.3.4", + "x_forwarded_proto" => "https", + "x_forwarded_host" => "evil.example.com", + "x_forwarded_port" => "8443", + ] + request = Protocol::HTTP::Request.new("http", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1")) + proxy.prepare_request(request, host) + expect(request.headers["x_forwarded_for"]).to be_nil + expect(request.headers["x_forwarded_proto"]).to be_nil + expect(request.headers["x_forwarded_host"]).to be_nil + expect(request.headers["x_forwarded_port"]).to be_nil + expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"] + expect(request.headers["x-forwarded-proto"]).to be == ["http"] + expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=http"] + end + it "doesn't let connection tokens strip authored forwarding headers" do + headers = Protocol::HTTP::Headers[ + "connection" => "x-forwarded-for, x-forwarded-proto, forwarded, via", + ] + request = Protocol::HTTP::Request.new("http", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1")) + proxy.prepare_request(request, host) + expect(request.headers["connection"]).to be_nil + expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"] + expect(request.headers["x-forwarded-proto"]).to be == ["http"] + expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=http"] + expect(request.headers["via"]).not.to be_nil + end + it "formats IPv6 addresses according to RFC 7239" do + request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(Addrinfo.ip("::1")) + + proxy.prepare_request(request, host) + + # RFC 7239 requires IPv6 to be bracketed and quoted in `Forwarded`... + expect(request.headers["forwarded"]).to be == ["for=\"[::1]\";proto=https"] + # ...but `X-Forwarded-For` carries the bare address. + expect(request.headers["x-forwarded-for"]).to be == ["::1"] + end + + it "omits the client address when the remote address is unavailable" do + request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil) + expect(request).to receive(:remote_address).and_return(nil) + + proxy.prepare_request(request, host) + + # With no remote address there is nothing to author, so neither the legacy + # `x-forwarded-for` nor a `for=` element in `forwarded` is emitted. + expect(request.headers["x-forwarded-for"]).to be_nil + expect(request.headers["forwarded"]).to be == ["proto=https"] + + # The scheme and via are still authored from connection-level facts. + expect(request.headers["x-forwarded-proto"]).to be == ["https"] + expect(request.headers["via"]).not.to be_nil + end + it "defers if no host is available" do request = Protocol::HTTP::Request.new("www.groogle.com", "GET", "/", nil, headers, nil)