Don't return proxied redirects to the client

This commit is contained in:
Tim Allclair 2020-06-17 11:09:02 -07:00
parent 82baa26905
commit 1f991f8a2f
4 changed files with 62 additions and 9 deletions

View File

@ -453,7 +453,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.

View File

@ -331,13 +331,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"},

View File

@ -304,6 +304,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)

View File

@ -512,7 +512,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")
@ -560,7 +560,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()
@ -620,6 +620,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,