Skip to content

Commit b77bae7

Browse files
benni-roggeclaude
andcommitted
Strip and re-author forwarding headers in Falcon::Middleware::Proxy.
As the trust boundary, Falcon must not trust client-supplied forwarding headers. `prepare_request` now discards every inbound `Forwarded` / `X-Forwarded-*` header and authors its own from connection-level facts, preventing a client from spoofing the forwarded address or scheme (CWE-290 / CWE-348). We emit both the modern RFC 7239 `Forwarded` header (with correctly bracketed and quoted IPv6) and the legacy `X-Forwarded-For` / `X-Forwarded-Proto` headers. The legacy headers are retained because they are still consumed downstream -- notably Rack's `Rack::Request#forwarded_for` (used by Rails' `ActionDispatch::RemoteIp`) falls back to `X-Forwarded-For`, and older Rack (< 3) and direct readers depend on it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e7007f6 commit b77bae7

2 files changed

Lines changed: 110 additions & 4 deletions

File tree

lib/falcon/middleware/proxy.rb

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,26 @@ class Proxy < Protocol::HTTP::Middleware
3838
VIA = "via"
3939
CONNECTION = "connection"
4040

41+
# Forwarding headers which carry trust-sensitive information about the
42+
# original client (their address and the request scheme). Because Falcon
43+
# acts as the trust boundary, any client-supplied values are untrustworthy,
44+
# so we strip every inbound forwarding header and author our own below from
45+
# connection-level facts.
46+
#
47+
# We emit both the modern RFC 7239 {FORWARDED} header and the legacy
48+
# `x-forwarded-for` / `x-forwarded-proto` headers, because many downstream
49+
# consumers still read the legacy ones. Notably Rack's
50+
# `Rack::Request#forwarded_for` (used by Rails' `ActionDispatch::RemoteIp`)
51+
# only prefers `Forwarded` and falls back to `X-Forwarded-For`; older Rack
52+
# (< 3) and a lot of application code read `X-Forwarded-For` directly.
53+
FORWARDING_HEADERS = [
54+
FORWARDED,
55+
X_FORWARDED_FOR,
56+
X_FORWARDED_PROTO,
57+
"x-forwarded-host",
58+
"x-forwarded-port",
59+
]
60+
4161
# HTTP hop headers which *should* not be passed through the proxy.
4262
HOP_HEADERS = [
4363
"connection",
@@ -101,10 +121,13 @@ def prepare_headers(headers)
101121
end
102122

103123
# Prepare the request to be proxied to the specified host.
104-
# In particular, we set appropriate {VIA}, {FORWARDED}, {X_FORWARDED_FOR} and {X_FORWARDED_PROTO} headers.
124+
#
125+
# Falcon acts as the trust boundary, so we strip any client-supplied
126+
# {FORWARDING_HEADERS} and author our own from connection-level facts: the
127+
# RFC 7239 {FORWARDED} header plus the legacy {X_FORWARDED_FOR} /
128+
# {X_FORWARDED_PROTO} headers, along with an appended {VIA} header. This
129+
# prevents a client from spoofing the forwarded address or scheme.
105130
def prepare_request(request, host)
106-
forwarded = []
107-
108131
Console.debug(self) do |buffer|
109132
buffer.puts "Request authority: #{request.authority}"
110133
buffer.puts "Host authority: #{host.authority}"
@@ -115,9 +138,14 @@ def prepare_request(request, host)
115138
# 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.
116139
request.authority = host.authority
117140

141+
# Discard any inbound forwarding headers so a client can't spoof them; we author our own below from connection-level facts.
142+
request.headers.extract(FORWARDING_HEADERS)
143+
144+
forwarded = []
145+
118146
if address = request.remote_address
119147
request.headers.add(X_FORWARDED_FOR, address.ip_address)
120-
forwarded << "for=#{address.ip_address}"
148+
forwarded << "for=#{forwarded_node(address)}"
121149
end
122150

123151
if scheme = request.scheme
@@ -136,6 +164,18 @@ def prepare_request(request, host)
136164
return request
137165
end
138166

167+
# Format a remote address as an RFC 7239 `for=` node identifier.
168+
# IPv6 addresses must be enclosed in square brackets and quoted.
169+
# @parameter address [Addrinfo] The remote address of the client.
170+
# @returns [String] The node identifier for use in a {FORWARDED} header.
171+
def forwarded_node(address)
172+
if address.ipv6?
173+
"\"[#{address.ip_address}]\""
174+
else
175+
address.ip_address
176+
end
177+
end
178+
139179
# Proxy the request if the authority matches a specific host.
140180
# @parameter request [Protocol::HTTP::Request]
141181
def call(request)

test/falcon/middleware/proxy.rb

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def proxy_for(**options)
2727

2828
let(:headers) {Protocol::HTTP::Headers["accept" => "*/*"]}
2929

30+
let(:host) {proxy_for(authority: "www.google.com", endpoint: Async::HTTP::Endpoint.parse("https://www.google.com"))}
31+
3032
it "removes proxy authorization by default" do
3133
headers = Protocol::HTTP::Headers[
3234
"authorization" => "Bearer application",
@@ -47,11 +49,75 @@ def proxy_for(**options)
4749

4850
expect(response).not.to be(:failure?)
4951

52+
expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=https"]
5053
expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"]
5154

5255
proxy.close
5356
end
5457

58+
it "authors a forwarded header from the connection" do
59+
request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil)
60+
expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1"))
61+
62+
proxy.prepare_request(request, host)
63+
64+
expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=https"]
65+
expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"]
66+
expect(request.headers["x-forwarded-proto"]).to be == ["https"]
67+
expect(request.headers["via"]).not.to be_nil
68+
end
69+
70+
it "strips client-supplied forwarding headers so they can't be spoofed" do
71+
headers = Protocol::HTTP::Headers[
72+
"x-forwarded-for" => "1.2.3.4",
73+
"x-forwarded-proto" => "https",
74+
"x-forwarded-host" => "evil.example.com",
75+
"x-forwarded-port" => "8443",
76+
"forwarded" => "for=9.9.9.9;proto=https",
77+
]
78+
request = Protocol::HTTP::Request.new("http", "www.google.com", "GET", "/", nil, headers, nil)
79+
expect(request).to receive(:remote_address).and_return(Addrinfo.ip("127.0.0.1"))
80+
81+
proxy.prepare_request(request, host)
82+
83+
# The spoofed values are stripped and replaced with Falcon's own.
84+
expect(request.headers["x-forwarded-for"]).to be == ["127.0.0.1"]
85+
expect(request.headers["x-forwarded-proto"]).to be == ["http"]
86+
expect(request.headers["forwarded"]).to be == ["for=127.0.0.1;proto=http"]
87+
88+
# `x-forwarded-host` and `x-forwarded-port` are stripped and not re-authored, so they can't be spoofed.
89+
expect(request.headers["x-forwarded-host"]).to be_nil
90+
expect(request.headers["x-forwarded-port"]).to be_nil
91+
end
92+
93+
it "formats IPv6 addresses according to RFC 7239" do
94+
request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil)
95+
expect(request).to receive(:remote_address).and_return(Addrinfo.ip("::1"))
96+
97+
proxy.prepare_request(request, host)
98+
99+
# RFC 7239 requires IPv6 to be bracketed and quoted in `Forwarded`...
100+
expect(request.headers["forwarded"]).to be == ["for=\"[::1]\";proto=https"]
101+
# ...but `X-Forwarded-For` carries the bare address.
102+
expect(request.headers["x-forwarded-for"]).to be == ["::1"]
103+
end
104+
105+
it "omits the client address when the remote address is unavailable" do
106+
request = Protocol::HTTP::Request.new("https", "www.google.com", "GET", "/", nil, headers, nil)
107+
expect(request).to receive(:remote_address).and_return(nil)
108+
109+
proxy.prepare_request(request, host)
110+
111+
# With no remote address there is nothing to author, so neither the legacy
112+
# `x-forwarded-for` nor a `for=` element in `forwarded` is emitted.
113+
expect(request.headers["x-forwarded-for"]).to be_nil
114+
expect(request.headers["forwarded"]).to be == ["proto=https"]
115+
116+
# The scheme and via are still authored from connection-level facts.
117+
expect(request.headers["x-forwarded-proto"]).to be == ["https"]
118+
expect(request.headers["via"]).not.to be_nil
119+
end
120+
55121
it "defers if no host is available" do
56122
request = Protocol::HTTP::Request.new("www.groogle.com", "GET", "/", nil, headers, nil)
57123

0 commit comments

Comments
 (0)