diff --git a/lib/falcon/middleware/proxy.rb b/lib/falcon/middleware/proxy.rb index aafd1a70..31dec39d 100644 --- a/lib/falcon/middleware/proxy.rb +++ b/lib/falcon/middleware/proxy.rb @@ -36,14 +36,21 @@ class Proxy < Protocol::HTTP::Middleware VIA = "via" CONNECTION = "connection" + HOST = "host" - # HTTP hop headers which *should* not be passed through the proxy. + TE = "te" + TRAILERS = "trailers" + + # Connection-specific fields which intermediaries must remove before forwarding (RFC 9110 Section 7.6.1), plus the legacy proxy-authentication fields. HOP_HEADERS = [ "connection", "keep-alive", - "public", "proxy-authenticate", "proxy-authorization", + "proxy-connection", + "public", + "te", + "trailer", "transfer-encoding", "upgrade", ] @@ -90,13 +97,22 @@ def lookup(request) end # Prepare the headers to be sent to an upstream host. - # In particular, we delete all connection and hop headers. + # In particular, we delete all connection and hop headers, but preserve `te: trailers` if the client sent it. + # @parameter headers [Protocol::HTTP::Headers] The request headers, modified in place. def prepare_headers(headers) + # Capture `te` before extraction, as a compliant HTTP/1.1 client also lists it in `connection`: + te = headers[TE] + if connection = headers[CONNECTION] headers.extract(connection) end headers.extract(HOP_HEADERS) + + # `te: trailers` is preserved since request trailers are forwarded end-to-end, and gRPC requires it: + if te&.trailers? + headers.add(TE, TRAILERS) + end end # Prepare the request to be proxied to the specified host. @@ -114,6 +130,9 @@ 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 + # The upstream connection derives its own `host` from `request.authority`: + request.headers.delete(HOST) + if address = request.remote_address request.headers.add(X_FORWARDED_FOR, address.ip_address) forwarded << "for=#{address.ip_address}" diff --git a/releases.md b/releases.md index f1710013..b1cfd2f7 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,10 @@ # Releases +## Unreleased + + - Strip `te`, `trailer` and `proxy-connection` hop-by-hop headers in `Falcon::Middleware::Proxy`, preserving `te: trailers` which is permitted over HTTP/2 and required by gRPC. + - Remove the `host` header in `Falcon::Middleware::Proxy` when rewriting the request authority, so HTTP/2 upstreams no longer receive a `host` that disagrees with `:authority`. + ## v0.57.0 - Update the Envoy cluster example to use dedicated CDS and EDS services from `async-service-supervisor-envoy` v0.5. diff --git a/test/falcon/middleware/proxy.rb b/test/falcon/middleware/proxy.rb index ab948373..85691adc 100644 --- a/test/falcon/middleware/proxy.rb +++ b/test/falcon/middleware/proxy.rb @@ -39,6 +39,74 @@ def proxy_for(**options) expect(headers["proxy-authorization"]).to be_nil end + it "removes hop-by-hop headers" do + headers = Protocol::HTTP::Headers[ + "accept" => "*/*", + "te" => "gzip, trailers", + "trailer" => "etag", + "proxy-connection" => "keep-alive", + "upgrade" => "h2c", + ] + proxy.prepare_headers(headers) + expect(headers["accept"]).to be == ["*/*"] + expect(headers["te"]).to be == ["trailers"] + expect(headers["trailer"]).to be_nil + expect(headers["proxy-connection"]).to be_nil + expect(headers["upgrade"]).to be_nil + end + + it "removes te if it does not include trailers" do + headers = Protocol::HTTP::Headers[ + "te" => "gzip", + ] + proxy.prepare_headers(headers) + expect(headers["te"]).to be_nil + end + + it "preserves te: trailers" do + headers = Protocol::HTTP::Headers[ + "te" => "trailers", + ] + proxy.prepare_headers(headers) + expect(headers["te"]).to be == ["trailers"] + end + + it "preserves te: trailers when te is listed in the connection header" do + headers = Protocol::HTTP::Headers[ + "connection" => "te", + "te" => "trailers", + ] + proxy.prepare_headers(headers) + expect(headers["connection"]).to be_nil + expect(headers["te"]).to be == ["trailers"] + end + + it "removes headers named by the connection header" do + headers = Protocol::HTTP::Headers[ + "accept" => "*/*", + "connection" => "x-custom-hop", + "x-custom-hop" => "value", + ] + proxy.prepare_headers(headers) + expect(headers["accept"]).to be == ["*/*"] + expect(headers["connection"]).to be_nil + expect(headers["x-custom-hop"]).to be_nil + end + + it "removes headers named by the connection header regardless of case" do + headers = Protocol::HTTP::Headers[ + "Accept" => "*/*", + "Connection" => "TE, Keep-Alive", + "TE" => "gzip", + "Keep-Alive" => "timeout=5", + ] + proxy.prepare_headers(headers) + expect(headers["accept"]).to be == ["*/*"] + expect(headers["connection"]).to be_nil + expect(headers["te"]).to be_nil + expect(headers["keep-alive"]).to be_nil + end + it "can select client based on authority" do request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil) @@ -79,6 +147,19 @@ def proxy_for(**options) expect(response.read).to be == "RuntimeError" end + it "removes the host header when rewriting the authority" do + headers = Protocol::HTTP::Headers["host" => "www.google.com"] + request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", "HTTP/2", headers, nil) + host = proxy_for(authority: "upstream.example.com", endpoint: Async::HTTP::Endpoint.parse("https://upstream.example.com")) + + expect(request).to receive(:remote_address).and_return(nil) + + proxy.prepare_request(request, host) + + expect(request.authority).to be == "upstream.example.com" + expect(request.headers["host"]).to be_nil + end + it "logs proxy request details when preparing requests" do request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", "HTTP/1.1", headers, nil) host = proxy_for(authority: "www.google.com", endpoint: Async::HTTP::Endpoint.parse("https://www.google.com"))