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
|
From ba3ca4929ed3887c95f94fcf97610f3449446804 Mon Sep 17 00:00:00 2001
From: Tim Allclair <tallclair@google.com>
Date: Wed, 17 Jun 2020 11:09:02 -0700
Subject: [PATCH] Don't return proxied redirects to the client
CVE: CVE-2020-8559
Upstream-Status: Backport [https://github.com/kubernetes/kubernetes.git branch:release-1.16]
Signed-off-by: Zhixiong Chi <zhixiong.chi@windriver.com>
---
.../k8s.io/apimachinery/pkg/util/net/http.go | 2 +-
.../apimachinery/pkg/util/net/http_test.go | 12 ++---
.../pkg/util/proxy/upgradeaware.go | 10 ++++
.../pkg/util/proxy/upgradeaware_test.go | 47 ++++++++++++++++++-
4 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http.go b/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http.go
index bd79d6c4a09..c24fbc6921c 100644
--- a/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http.go
+++ b/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http.go
@@ -431,7 +431,7 @@ redirectLoop:
// Only follow redirects to the same host. Otherwise, propagate the redirect response back.
if requireSameHostRedirects && location.Hostname() != originalLocation.Hostname() {
- break redirectLoop
+ return nil, nil, fmt.Errorf("hostname mismatch: expected %s, found %s", originalLocation.Hostname(), location.Hostname())
}
// Reset the connection.
diff --git a/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go b/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go
index 4e4e317b9a4..142b80f1a84 100644
--- a/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go
+++ b/src/import/staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go
@@ -330,13 +330,13 @@ func TestConnectWithRedirects(t *testing.T) {
redirects: []string{"/1", "/2", "/3", "/4", "/5", "/6", "/7", "/8", "/9", "/10"},
expectError: true,
}, {
- desc: "redirect to different host are prevented",
- redirects: []string{"http://example.com/foo"},
- expectedRedirects: 0,
+ desc: "redirect to different host are prevented",
+ redirects: []string{"http://example.com/foo"},
+ expectError: true,
}, {
- desc: "multiple redirect to different host forbidden",
- redirects: []string{"/1", "/2", "/3", "http://example.com/foo"},
- expectedRedirects: 3,
+ desc: "multiple redirect to different host forbidden",
+ redirects: []string{"/1", "/2", "/3", "http://example.com/foo"},
+ expectError: true,
}, {
desc: "redirect to different port is allowed",
redirects: []string{"http://HOST/foo"},
diff --git a/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go b/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
index fcdc76a0529..3a02919d135 100644
--- a/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
+++ b/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware.go
@@ -298,6 +298,16 @@ func (h *UpgradeAwareHandler) tryUpgrade(w http.ResponseWriter, req *http.Reques
rawResponse = headerBytes
}
+ // If the backend did not upgrade the request, return an error to the client. If the response was
+ // an error, the error is forwarded directly after the connection is hijacked. Otherwise, just
+ // return a generic error here.
+ if backendHTTPResponse.StatusCode != http.StatusSwitchingProtocols && backendHTTPResponse.StatusCode < 400 {
+ err := fmt.Errorf("invalid upgrade response: status code %d", backendHTTPResponse.StatusCode)
+ klog.Errorf("Proxy upgrade error: %v", err)
+ h.Responder.Error(w, req, err)
+ return true
+ }
+
// Once the connection is hijacked, the ErrorResponder will no longer work, so
// hijacking should be the last step in the upgrade.
requestHijacker, ok := w.(http.Hijacker)
diff --git a/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go b/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go
index 7d14f6534a8..236362373cd 100644
--- a/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go
+++ b/src/import/staging/src/k8s.io/apimachinery/pkg/util/proxy/upgradeaware_test.go
@@ -493,7 +493,7 @@ func (r *noErrorsAllowed) Error(w http.ResponseWriter, req *http.Request, err er
r.t.Error(err)
}
-func TestProxyUpgradeErrorResponse(t *testing.T) {
+func TestProxyUpgradeConnectionErrorResponse(t *testing.T) {
var (
responder *fakeResponder
expectedErr = errors.New("EXPECTED")
@@ -541,7 +541,7 @@ func TestProxyUpgradeErrorResponse(t *testing.T) {
func TestProxyUpgradeErrorResponseTerminates(t *testing.T) {
for _, intercept := range []bool{true, false} {
- for _, code := range []int{200, 400, 500} {
+ for _, code := range []int{400, 500} {
t.Run(fmt.Sprintf("intercept=%v,code=%v", intercept, code), func(t *testing.T) {
// Set up a backend server
backend := http.NewServeMux()
@@ -601,6 +601,49 @@ func TestProxyUpgradeErrorResponseTerminates(t *testing.T) {
}
}
+func TestProxyUpgradeErrorResponse(t *testing.T) {
+ for _, intercept := range []bool{true, false} {
+ for _, code := range []int{200, 300, 302, 307} {
+ t.Run(fmt.Sprintf("intercept=%v,code=%v", intercept, code), func(t *testing.T) {
+ // Set up a backend server
+ backend := http.NewServeMux()
+ backend.Handle("/hello", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ http.Redirect(w, r, "https://example.com/there", code)
+ }))
+ backendServer := httptest.NewServer(backend)
+ defer backendServer.Close()
+ backendServerURL, _ := url.Parse(backendServer.URL)
+ backendServerURL.Path = "/hello"
+
+ // Set up a proxy pointing to a specific path on the backend
+ proxyHandler := NewUpgradeAwareHandler(backendServerURL, nil, false, false, &fakeResponder{t: t})
+ proxyHandler.InterceptRedirects = intercept
+ proxyHandler.RequireSameHostRedirects = true
+ proxy := httptest.NewServer(proxyHandler)
+ defer proxy.Close()
+ proxyURL, _ := url.Parse(proxy.URL)
+
+ conn, err := net.Dial("tcp", proxyURL.Host)
+ require.NoError(t, err)
+ bufferedReader := bufio.NewReader(conn)
+
+ // Send upgrade request resulting in a non-101 response from the backend
+ req, _ := http.NewRequest("GET", "/", nil)
+ req.Header.Set(httpstream.HeaderConnection, httpstream.HeaderUpgrade)
+ require.NoError(t, req.Write(conn))
+ // Verify we get the correct response and full message body content
+ resp, err := http.ReadResponse(bufferedReader, nil)
+ require.NoError(t, err)
+ assert.Equal(t, fakeStatusCode, resp.StatusCode)
+ resp.Body.Close()
+
+ // clean up
+ conn.Close()
+ })
+ }
+ }
+}
+
func TestDefaultProxyTransport(t *testing.T) {
tests := []struct {
name,
--
2.17.0
|
