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

Author: dunglas

src/net/http/httputil/reverseproxy.go

@@ -25,10 +25,14 @@ import (
 // sends it to another server, proxying the response back to the
 // client.
 //
-// ReverseProxy automatically sets the X-Forwarded-For,
-// X-Forwarded-Host and X-Forwarded-Proto headers.
-// Previous values of these headers can be preversed by
-// setting TrustForwardedHeaders to true.
+// ReverseProxy automatically sets the client IP as the value of the
+// X-Forwarded-For header.
+// If an X-Forwarded-For header already exists, the client IP is
+// appended to the existing values.
+// To prevent IP spoofing, be sure to delete any pre-existing
+// X-Forwarded-For header coming from the client or
+// an untrusted proxy, for instance, by setting
+// OverwriteForwardedHeaders to true.
 type ReverseProxy struct {
 	// Director must be a function which modifies
 	// the request into a new request to be sent
@@ -82,19 +86,19 @@ type ReverseProxy struct {
 	// a 502 Status Bad Gateway response.
 	ErrorHandler func(http.ResponseWriter, *http.Request, error)
 
-	// TrustForwardedHeaders specifies if X-Forwarded-For,
+	// OverwriteForwardedHeaders specifies if X-Forwarded-For,
 	// X-Forwarded-Proto and X-Forwarded-Host headers coming from
-	// the previous proxy must be trusted or not.
+	// the previous proxy must be replaced or not.
 	//
-	// If true, existing values of X-Forwarded-Proto and
-	// X-Forwarded-Host will be preserved, and the current client IP
-	// will be appended to the list in X-Forwarded-For. In this case
-	// be sure that these 3 headers are removed from the request if
-	// sent by the client to prevent spoofing attacks.
+	// If true, these headers 3 headers will be set regardless of any
+	// existing value.
 	//
-	// If false, values of these headers will be set regardless of
-	// any existing value.
-	TrustForwardedHeaders bool
+	// If false, X-Forwarded-Proto and X-Forwarded-Host will not be
+	// touched (not even created if they don't exist), and the
+	// current client IP will be appended to the list in
+	// X-Forwarded-For. If X-Forwarded-For doesn't exist, it will be
+	// created.
+	OverwriteForwardedHeaders bool
 }
 
 // A BufferPool is an interface for getting and returning temporary
@@ -255,20 +259,14 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		outreq.Header.Set("Upgrade", reqUpType)
 	}
 
-	if _, ok := outreq.Header["X-Forwarded-Proto"]; !ok || !p.TrustForwardedHeaders {
+	if p.OverwriteForwardedHeaders {
 		proto := "https"
 		if req.TLS == nil {
 			proto = "http"
 		}
 
 		outreq.Header.Set("X-Forwarded-Proto", proto)
-	}
-
-	if _, ok := outreq.Header["X-Forwarded-Host"]; !ok || !p.TrustForwardedHeaders {
 		outreq.Header.Set("X-Forwarded-Host", outreq.Host)
-	}
-
-	if !p.TrustForwardedHeaders {
 		outreq.Header.Del("X-Forwarded-For")
 	}
 

src/net/http/httputil/reverseproxy_test.go

@@ -236,24 +236,16 @@ func TestReverseProxyStripHeadersPresentInConnection(t *testing.T) {
 	}
 }
 
-func TestDontTrustForwardedHeaders(t *testing.T) {
+func TestXForwardedFor(t *testing.T) {
 	const prevForwardedFor = "client ip"
-	const prevForwardedProto = "https"
-	const prevForwardedHost = "example.com"
 	const backendResponse = "I am the backend"
 	const backendStatus = 404
 	backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Header.Get("X-Forwarded-For") == "" {
 			t.Errorf("didn't get X-Forwarded-For header")
 		}
-		if strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
-			t.Errorf("X-Forwarded-For contains prior data")
-		}
-		if strings.Contains(r.Header.Get("X-Forwarded-Proto"), prevForwardedProto) {
-			t.Errorf("X-Forwarded-Proto contains prior data")
-		}
-		if strings.Contains(r.Header.Get("X-Forwarded-Host"), prevForwardedHost) {
-			t.Errorf("X-Forwarded-Host contains prior data")
+		if !strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
+			t.Errorf("X-Forwarded-For didn't contain prior data")
 		}
 		w.WriteHeader(backendStatus)
 		w.Write([]byte(backendResponse))
@@ -271,8 +263,6 @@ func TestDontTrustForwardedHeaders(t *testing.T) {
 	getReq.Host = "some-name"
 	getReq.Header.Set("Connection", "close")
 	getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
-	getReq.Header.Set("X-Forwarded-Proto", prevForwardedProto)
-	getReq.Header.Set("X-Forwarded-Host", prevForwardedHost)
 	getReq.Close = true
 	res, err := frontend.Client().Do(getReq)
 	if err != nil {
@@ -287,7 +277,7 @@ func TestDontTrustForwardedHeaders(t *testing.T) {
 	}
 }
 
-func TestXForwardedFor(t *testing.T) {
+func TestOverwriteForwardedHeaders(t *testing.T) {
 	const prevForwardedFor = "client ip"
 	const prevForwardedProto = "https"
 	const prevForwardedHost = "example.com"
@@ -297,14 +287,14 @@ func TestXForwardedFor(t *testing.T) {
 		if r.Header.Get("X-Forwarded-For") == "" {
 			t.Errorf("didn't get X-Forwarded-For header")
 		}
-		if !strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
-			t.Errorf("X-Forwarded-For didn't contain prior data")
+		if strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
+			t.Errorf("X-Forwarded-For contains prior data")
 		}
-		if !strings.Contains(r.Header.Get("X-Forwarded-Proto"), prevForwardedProto) {
-			t.Errorf("X-Forwarded-Proto didn't contain prior data")
+		if strings.Contains(r.Header.Get("X-Forwarded-Proto"), prevForwardedProto) {
+			t.Errorf("X-Forwarded-Proto contains prior data")
 		}
-		if !strings.Contains(r.Header.Get("X-Forwarded-Host"), prevForwardedHost) {
-			t.Errorf("X-Forwarded-Host didn't contain prior data")
+		if strings.Contains(r.Header.Get("X-Forwarded-Host"), prevForwardedHost) {
+			t.Errorf("X-Forwarded-Host contains prior data")
 		}
 		w.WriteHeader(backendStatus)
 		w.Write([]byte(backendResponse))
@@ -315,7 +305,7 @@ func TestXForwardedFor(t *testing.T) {
 		t.Fatal(err)
 	}
 	proxyHandler := NewSingleHostReverseProxy(backendURL)
-	proxyHandler.TrustForwardedHeaders = true
+	proxyHandler.OverwriteForwardedHeaders = true
 	frontend := httptest.NewServer(proxyHandler)
 	defer frontend.Close()