net/http/httputil: always remove hop-by-hop headers

Previously, we'd fail to remove the Connection header from a request
like this:

    Connection:
    Connection: x-header

Fixes #46313
Fixes CVE-2021-33197

Change-Id: Ie3009e926ceecfa86dfa6bcc6fe14ff01086be7d
Reviewed-on: https://go-review.googlesource.com/c/go/+/321929
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
This commit is contained in:
Filippo Valsorda 2021-05-21 14:02:30 -04:00
parent 9bc52686da
commit 950fa11c4c
2 changed files with 70 additions and 15 deletions

View file

@ -91,8 +91,9 @@ func TestReverseProxy(t *testing.T) {
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
getReq.Host = "some-name"
getReq.Header.Set("Connection", "close")
getReq.Header.Set("Te", "trailers")
getReq.Header.Set("Connection", "close, TE")
getReq.Header.Add("Te", "foo")
getReq.Header.Add("Te", "bar, trailers")
getReq.Header.Set("Proxy-Connection", "should be deleted")
getReq.Header.Set("Upgrade", "foo")
getReq.Close = true
@ -236,6 +237,64 @@ func TestReverseProxyStripHeadersPresentInConnection(t *testing.T) {
}
}
func TestReverseProxyStripEmptyConnection(t *testing.T) {
// See Issue 46313.
const backendResponse = "I am the backend"
// someConnHeader is some arbitrary header to be declared as a hop-by-hop header
// in the Request's Connection header.
const someConnHeader = "X-Some-Conn-Header"
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if c := r.Header.Values("Connection"); len(c) != 0 {
t.Errorf("handler got header %q = %v; want empty", "Connection", c)
}
if c := r.Header.Get(someConnHeader); c != "" {
t.Errorf("handler got header %q = %q; want empty", someConnHeader, c)
}
w.Header().Add("Connection", "")
w.Header().Add("Connection", someConnHeader)
w.Header().Set(someConnHeader, "should be deleted")
io.WriteString(w, backendResponse)
}))
defer backend.Close()
backendURL, err := url.Parse(backend.URL)
if err != nil {
t.Fatal(err)
}
proxyHandler := NewSingleHostReverseProxy(backendURL)
frontend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
proxyHandler.ServeHTTP(w, r)
if c := r.Header.Get(someConnHeader); c != "should be deleted" {
t.Errorf("handler modified header %q = %q; want %q", someConnHeader, c, "should be deleted")
}
}))
defer frontend.Close()
getReq, _ := http.NewRequest("GET", frontend.URL, nil)
getReq.Header.Add("Connection", "")
getReq.Header.Add("Connection", someConnHeader)
getReq.Header.Set(someConnHeader, "should be deleted")
res, err := frontend.Client().Do(getReq)
if err != nil {
t.Fatalf("Get: %v", err)
}
defer res.Body.Close()
bodyBytes, err := io.ReadAll(res.Body)
if err != nil {
t.Fatalf("reading body: %v", err)
}
if got, want := string(bodyBytes), backendResponse; got != want {
t.Errorf("got body %q; want %q", got, want)
}
if c := res.Header.Get("Connection"); c != "" {
t.Errorf("handler got header %q = %q; want empty", "Connection", c)
}
if c := res.Header.Get(someConnHeader); c != "" {
t.Errorf("handler got header %q = %q; want empty", someConnHeader, c)
}
}
func TestXForwardedFor(t *testing.T) {
const prevForwardedFor = "client ip"
const backendResponse = "I am the backend"