1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
|
From cbd1ca84453fecf3825a6bb9f985823e8bc32b76 Mon Sep 17 00:00:00 2001
From: Filippo Valsorda <filippo@golang.org>
Date: Fri, 21 May 2021 14:02:30 -0400
Subject: [PATCH] [release-branch.go1.15] 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
Updates #46313
Fixes #46314
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>
Reviewed-on: https://go-review.googlesource.com/c/go/+/323091
Run-TryBot: Katie Hockman <katie@golang.org>
Upstream-Status: Backport
CVE: CVE-2021-33197
Signed-off-by: Armin Kuster <akuster@mvista.com>
---
src/net/http/httputil/reverseproxy.go | 22 ++++----
src/net/http/httputil/reverseproxy_test.go | 63 +++++++++++++++++++++-
2 files changed, 70 insertions(+), 15 deletions(-)
Index: go/src/net/http/httputil/reverseproxy.go
===================================================================
--- go.orig/src/net/http/httputil/reverseproxy.go
+++ go/src/net/http/httputil/reverseproxy.go
@@ -221,22 +221,18 @@ func (p *ReverseProxy) ServeHTTP(rw http
// important is "Connection" because we want a persistent
// connection, regardless of what the client sent to us.
for _, h := range hopHeaders {
- hv := outreq.Header.Get(h)
- if hv == "" {
- continue
- }
- if h == "Te" && hv == "trailers" {
- // Issue 21096: tell backend applications that
- // care about trailer support that we support
- // trailers. (We do, but we don't go out of
- // our way to advertise that unless the
- // incoming client request thought it was
- // worth mentioning)
- continue
- }
outreq.Header.Del(h)
}
+ // Issue 21096: tell backend applications that care about trailer support
+ // that we support trailers. (We do, but we don't go out of our way to
+ // advertise that unless the incoming client request thought it was worth
+ // mentioning.) Note that we look at req.Header, not outreq.Header, since
+ // the latter has passed through removeConnectionHeaders.
+ if httpguts.HeaderValuesContainsToken(req.Header["Te"], "trailers") {
+ outreq.Header.Set("Te", "trailers")
+ }
+
// After stripping all the hop-by-hop connection headers above, add back any
// necessary for protocol upgrades, such as for websockets.
if reqUpType != "" {
Index: go/src/net/http/httputil/reverseproxy_test.go
===================================================================
--- go.orig/src/net/http/httputil/reverseproxy_test.go
+++ go/src/net/http/httputil/reverseproxy_test.go
@@ -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 TestReverseProxyStripHeadersPresent
}
}
+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 := ioutil.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"
|
