net/http/httputil: add support for X-Forwarded-Proto, X-Forwarded-Host and an option to not trust forwarded headers in ReverseProxy #36678

dunglas · 2020-01-21T17:05:33Z

In addition to X-Forwarded-For, which is already supported but undocumented
(#36672), this patch adds support for the X-Forwarded-Proto (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto)
and X-Forwarded-Host (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host)
headers in ReverseProxy.
It also introduces a new option, ReverseProxy.TrustForwardedHeaders
that must be explicitly set to true to trust forwarded headers coming from
the client or previous proxies.

Setting these headers is a standard behavior expected from reverse
proxies, especially when X-Forwarded-For is set (which is already the
case).

The new ReverseProxy.TrustForwardedHeaders option fixes a potential
common security issue in user land code. It forces the user to explicitly
opt-in to trust reverse proxies in front of the Go application using
ReverseProxy.

The current behavior of blindly trusting the X-Forwarded-For header
provided by the client is very dangerous.
For instance, because of the current (undocumented) behavior, the Vulcain
gateway server (https://vulcain.rocks) was sensible to a potential security
issue.

If this flag is not set to true, previous values set by the client or
untrusted proxies will be discarded (replaced by the values computed
by ReverseProxy).
Technically, this is a BC break, but, in my opinion, it is a necessary one
to improve the security of the whole Go ecosystem using this module.

Edit: added a backward compatible implementation, see #36678 (comment)

gopherbot · 2020-01-21T17:10:57Z

This PR (HEAD: 8648d02) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

ReverseProxy automatically sets the X-Forwarded-For header, if the request already contains a X-Forwarded-For header, the value of the client IP is appended to the existing header value. This behavior isn't documented anywhere, and can lead to IP spoofing security issues is the client is untrusted (the most common situation). This PR documents this behavior. For future versions, I proposed #36678 that implements a more secure default behavior and adds support for other forwarded headers. Change-Id: Ief14f5063caebfccb87714f54cffa927c714e5fd GitHub-Last-Rev: fd0bd29 GitHub-Pull-Request: #36672 Reviewed-on: https://go-review.googlesource.com/c/go/+/215617 Reviewed-by: Brad Fitzpatrick <[email protected]>

gopherbot · 2020-01-22T09:23:51Z

This PR (HEAD: c0b06fb) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

artyom · 2020-01-22T09:43:07Z

Can this be a documentation update, maybe with an example of httputil.ReverseProxy.Director function handling headers in question, instead of a backwards-incompatible change?

Note that even if this change is accepted, at least some ReverseProxy users would need to support pre-change and post-change Go versions, which adds maintenance overhead.

dunglas · 2020-01-22T12:44:37Z

@artyom Regarding TrustForwardedHeaders it's also possible to make the change backward compatible by setting TrustForwardedHeaders to true by default, or by renaming it OverrideForwardedHeaders, but IMHO it's less secure (as secure than currently actually), and it's always better to have to opt-in for potentially dangerous features (the current behavior has already be proven dangerous). Anyway, your call. I'm perfectly ok to make it backward compatible if you think it's better for the ecosystem.

Regarding setting X-Forwarded-Proto and X-Forwarded-Host, I think that it's a must have feature (especially because X-Forwarded-For is already set). It's the de facto standard, the behavior of most reverse proxies and the expected behavior of most web application frameworks, including Symfony, Laravel and Gorilla to name a few.

gopherbot · 2020-01-28T10:43:52Z

This PR (HEAD: a4f0f3e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

dunglas · 2020-01-28T10:44:10Z

@artyom I just pushed a new patch containing a backward compatible implementation. WDYT about this one?

gopherbot · 2020-01-28T10:47:04Z

This PR (HEAD: 64fd62d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

ruudk · 2020-04-02T15:37:40Z

src/net/http/httputil/reverseproxy.go

@@ -244,6 +259,17 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		outreq.Header.Set("Upgrade", reqUpType)
 	}

+	if p.OverwriteForwardedHeaders {
+		proto := "https"
+		if req.TLS == nil {


Not sure about this. req.TLS is only set when the current http server is handling the TLS. But if TLS is done by a load balancer in front of this server, it will say http, how can the backend know if it was an https request originally?

Then you'll have to handle this manually. If the TLS termination is handled by a non-HTTP server, the proxy cannot guess it. If it is handled by an HTTP server, then this server should set the Forwarded headers.

gopherbot · 2020-05-20T08:01:46Z

This PR (HEAD: 44b8b3d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

…t and an option to not trust forwarded headers in ReverseProxy

…verseProxy

gopherbot · 2022-01-06T16:10:09Z

Message from Kévin Dunglas:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-01-06T16:10:10Z

Message from Ian Lance Taylor:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-01-06T16:10:10Z

Message from Kévin Dunglas:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-01-06T16:10:11Z

Message from Damien Neil:

Patch Set 6:

(5 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-01-06T16:10:12Z

Message from Kévin Dunglas:

Patch Set 6:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2022-01-06T16:10:36Z

This PR (HEAD: aa0ff79) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/215637 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2022-01-06T16:14:25Z

Message from Kévin Dunglas:

Patch Set 7:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2023-01-25T23:09:15Z

Message from Damien Neil:

Patch Set 7:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/215637.
After addressing review feedback, remember to publish your drafts!

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jan 21, 2020

dunglas mentioned this pull request Jan 21, 2020

net/http/httputil: add docs about X-Forwarded-For in ReverseProxy #36672

Closed

dunglas force-pushed the x-forwarded branch from 8648d02 to c0b06fb Compare January 22, 2020 09:23

dunglas force-pushed the x-forwarded branch from c0b06fb to a4f0f3e Compare January 28, 2020 10:42

ruudk reviewed Apr 2, 2020

View reviewed changes

dunglas force-pushed the x-forwarded branch from 64fd62d to 44b8b3d Compare May 20, 2020 08:00

maximbaz mentioned this pull request Jun 15, 2020

Use http:// with fetch example mpolden/echoip#99

Merged

gopherbot force-pushed the master branch from a408139 to 5764653 Compare September 14, 2020 21:26

heschi closed this Dec 15, 2021

ianlancetaylor reopened this Dec 15, 2021

dunglas mentioned this pull request Jan 6, 2022

net/http/httputil: add X-Forwarded-Proto and X-Forwarded-Host by default #50465

Closed

dunglas and others added 5 commits January 6, 2022 15:27

net/http/httputil: add support for X-Forwarded-Proto, X-Forwarded-Hos…

840ec38

…t and an option to not trust forwarded headers in ReverseProxy

Improve docs

e10880d

net/http/httputil: add an option to overwrite forwarded headers in Re…

d657cdf

…verseProxy

typo

744aa33

implement proposal golang#50465

aa0ff79

dunglas force-pushed the x-forwarded branch from 44b8b3d to aa0ff79 Compare January 6, 2022 16:06

seankhliao closed this Feb 13, 2025

net/http/httputil: add support for X-Forwarded-Proto, X-Forwarded-Host and an option to not trust forwarded headers in ReverseProxy #36678

net/http/httputil: add support for X-Forwarded-Proto, X-Forwarded-Host and an option to not trust forwarded headers in ReverseProxy #36678

Uh oh!

Conversation

dunglas commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gopherbot commented Jan 21, 2020

Uh oh!

gopherbot commented Jan 22, 2020

Uh oh!

artyom commented Jan 22, 2020

Uh oh!

dunglas commented Jan 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gopherbot commented Jan 28, 2020

Uh oh!

dunglas commented Jan 28, 2020

Uh oh!

gopherbot commented Jan 28, 2020

Uh oh!

ruudk Apr 2, 2020

Choose a reason for hiding this comment

Uh oh!

dunglas Apr 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gopherbot commented May 20, 2020

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 6, 2022

Uh oh!

gopherbot commented Jan 25, 2023

Uh oh!

Uh oh!

dunglas commented Jan 21, 2020 •

edited

Loading

dunglas commented Jan 22, 2020 •

edited

Loading

dunglas Apr 2, 2020 •

edited

Loading