net/http/httputil: add X-Forwarded-{Host,Proto} headers in ReverseProxy

X-Forwarded-Host contains the original request's host.

X-Forwarded-Proto contains "http" or "https", depending on whether the
original request was made on a TLS-secured connection.

Setting either header to nil in Director disables adding the header,
same as for X-Forwarded-For.

Fixes #50465.

Change-Id: If8ed1f48d83f8ea0389c53519bc7994cb53891db
Reviewed-on: https://go-review.googlesource.com/c/go/+/407414
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index b5d3ce711097b1370a6ff6602041de686a4ab214..a5a3900fb37c86cb60621db23b9c72c270118e76 100644 (file)
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -28,14 +28,18 @@ import (
 // sends it to another server, proxying the response back to the
 // client.
 //
-// ReverseProxy by default sets the client IP as the value of the
-// X-Forwarded-For header.
+// ReverseProxy by default sets
+//   - the X-Forwarded-For header to the client IP address;
+//   - the X-Forwarded-Host header to the host of the original client
+//     request; and
+//   - the X-Forwarded-Proto header to "https" if the client request
+//     was made on a TLS-enabled connection or "http" otherwise.
 //
 // If an X-Forwarded-For header already exists, the client IP is
-// appended to the existing values. As a special case, if the header
-// exists in the Request.Header map but has a nil value (such as when
-// set by the Director func), the X-Forwarded-For header is
-// not modified.
+// appended to the existing values.
+//
+// If a header exists in the Request.Header map but has a nil value
+// (such as when set by the Director func), it is not modified.
 //
 // To prevent IP spoofing, be sure to delete any pre-existing
 // X-Forwarded-For header coming from the client or
@@ -306,6 +310,16 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
                        outreq.Header.Set("X-Forwarded-For", clientIP)
                }
        }
+       if prior, ok := outreq.Header["X-Forwarded-Host"]; !(ok && prior == nil) {
+               outreq.Header.Set("X-Forwarded-Host", req.Host)
+       }
+       if prior, ok := outreq.Header["X-Forwarded-Proto"]; !(ok && prior == nil) {
+               if req.TLS == nil {
+                       outreq.Header.Set("X-Forwarded-Proto", "http")
+               } else {
+                       outreq.Header.Set("X-Forwarded-Proto", "https")
+               }
+       }
 
        res, err := transport.RoundTrip(outreq)
        if err != nil {

diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index 90e8903e9cfd5bfa3086422cb9f442a0861bd250..23453c8bdc909e1e5fac8ad65bce94796cbe6dd5 100644 (file)
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -50,6 +50,12 @@ func TestReverseProxy(t *testing.T) {
                if r.Header.Get("X-Forwarded-For") == "" {
                        t.Errorf("didn't get X-Forwarded-For header")
                }
+               if r.Header.Get("X-Forwarded-Host") == "" {
+                       t.Errorf("didn't get X-Forwarded-Host header")
+               }
+               if r.Header.Get("X-Forwarded-Proto") == "" {
+                       t.Errorf("didn't get X-Forwarded-Proto header")
+               }
                if c := r.Header.Get("Connection"); c != "" {
                        t.Errorf("handler got Connection header value %q", c)
                }
@@ -299,6 +305,7 @@ func TestXForwardedFor(t *testing.T) {
        const prevForwardedFor = "client ip"
        const backendResponse = "I am the backend"
        const backendStatus = 404
+       const host = "some-name"
        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")
@@ -306,6 +313,12 @@ func TestXForwardedFor(t *testing.T) {
                if !strings.Contains(r.Header.Get("X-Forwarded-For"), prevForwardedFor) {
                        t.Errorf("X-Forwarded-For didn't contain prior data")
                }
+               if got, want := r.Header.Get("X-Forwarded-Host"), host; got != want {
+                       t.Errorf("X-Forwarded-Host = %q, want %q", got, want)
+               }
+               if got, want := r.Header.Get("X-Forwarded-Proto"), "http"; got != want {
+                       t.Errorf("X-Forwarded-Proto = %q, want %q", got, want)
+               }
                w.WriteHeader(backendStatus)
                w.Write([]byte(backendResponse))
        }))
@@ -319,7 +332,7 @@ func TestXForwardedFor(t *testing.T) {
        defer frontend.Close()
 
        getReq, _ := http.NewRequest("GET", frontend.URL, nil)
-       getReq.Host = "some-name"
+       getReq.Host = host
        getReq.Header.Set("Connection", "close")
        getReq.Header.Set("X-Forwarded-For", prevForwardedFor)
        getReq.Close = true
@@ -336,11 +349,36 @@ func TestXForwardedFor(t *testing.T) {
        }
 }
 
+func TestXForwardedProtoTLS(t *testing.T) {
+       backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+               if got, want := r.Header.Get("X-Forwarded-Proto"), "https"; got != want {
+                       t.Errorf("X-Forwarded-Proto = %q, want %q", got, want)
+               }
+       }))
+       defer backend.Close()
+       backendURL, err := url.Parse(backend.URL)
+       if err != nil {
+               t.Fatal(err)
+       }
+       proxyHandler := NewSingleHostReverseProxy(backendURL)
+       frontend := httptest.NewTLSServer(proxyHandler)
+       defer frontend.Close()
+
+       getReq, _ := http.NewRequest("GET", frontend.URL, nil)
+       getReq.Host = "some-host"
+       _, err = frontend.Client().Do(getReq)
+       if err != nil {
+               t.Fatalf("Get: %v", err)
+       }
+}
+
 // Issue 38079: don't append to X-Forwarded-For if it's present but nil
 func TestXForwardedFor_Omit(t *testing.T) {
        backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-               if v := r.Header.Get("X-Forwarded-For"); v != "" {
-                       t.Errorf("got X-Forwarded-For header: %q", v)
+               for _, h := range []string{"X-Forwarded-For", "X-Forwarded-Host", "X-Forwarded-Proto"} {
+                       if v := r.Header.Get(h); v != "" {
+                               t.Errorf("got %v header: %q", h, v)
+                       }
                }
                w.Write([]byte("hi"))
        }))
@@ -356,6 +394,8 @@ func TestXForwardedFor_Omit(t *testing.T) {
        oldDirector := proxyHandler.Director
        proxyHandler.Director = func(r *http.Request) {
                r.Header["X-Forwarded-For"] = nil
+               r.Header["X-Forwarded-Host"] = nil
+               r.Header["X-Forwarded-Proto"] = nil
                oldDirector(r)
        }
 
@@ -1029,13 +1069,16 @@ func TestClonesRequestHeaders(t *testing.T) {
        }
        rp.ServeHTTP(httptest.NewRecorder(), req)
 
-       if req.Header.Get("From-Director") == "1" {
-               t.Error("Director header mutation modified caller's request")
-       }
-       if req.Header.Get("X-Forwarded-For") != "" {
-               t.Error("X-Forward-For header mutation modified caller's request")
+       for _, h := range []string{
+               "From-Director",
+               "X-Forwarded-For",
+               "X-Forwarded-Host",
+               "X-Forwarded-Proto",
+       } {
+               if req.Header.Get(h) != "" {
+                       t.Errorf("%v header mutation modified caller's request", h)
+               }
        }
-
 }
 
 type roundTripperFunc func(req *http.Request) (*http.Response, error)