fix: stop redirecting DERP and replicasync http requests (#10752)

Fixes an issue where setting CODER_REDIRECT_TO_ACCESS_URL breaks use of multiple Coder server replicas for DERP traffic.
This commit is contained in:
Spike Curtis
2023-11-20 14:46:59 +04:00
committed by GitHub
parent 5c48cb4447
commit 5173bce5cc
5 changed files with 123 additions and 13 deletions
+20
View File
@@ -1922,6 +1922,18 @@ func redirectToAccessURL(handler http.Handler, accessURL *url.URL, tunnel bool,
http.Redirect(w, r, accessURL.String(), http.StatusTemporaryRedirect)
}
// Exception: DERP
// We use this endpoint when creating a DERP-mesh in the enterprise version to directly
// dial other Coderd derpers. Redirecting to the access URL breaks direct dial since the
// access URL will be load-balanced in a multi-replica deployment.
//
// It's totally fine to access DERP over TLS, but we also don't need to redirect HTTP to
// HTTPS as DERP is itself an encrypted protocol.
if isDERPPath(r.URL.Path) {
handler.ServeHTTP(w, r)
return
}
// Only do this if we aren't tunneling.
// If we are tunneling, we want to allow the request to go through
// because the tunnel doesn't proxy with TLS.
@@ -1949,6 +1961,14 @@ func redirectToAccessURL(handler http.Handler, accessURL *url.URL, tunnel bool,
})
}
func isDERPPath(p string) bool {
segments := strings.SplitN(p, "/", 3)
if len(segments) < 2 {
return false
}
return segments[1] == "derp"
}
// IsLocalhost returns true if the host points to the local machine. Intended to
// be called with `u.Hostname()`.
func IsLocalhost(host string) bool {
+53
View File
@@ -243,3 +243,56 @@ func TestRedirectHTTPToHTTPSDeprecation(t *testing.T) {
})
}
}
func TestIsDERPPath(t *testing.T) {
t.Parallel()
testcases := []struct {
path string
expected bool
}{
//{
// path: "/derp",
// expected: true,
//},
{
path: "/derp/",
expected: true,
},
{
path: "/derp/latency-check",
expected: true,
},
{
path: "/derp/latency-check/",
expected: true,
},
{
path: "",
expected: false,
},
{
path: "/",
expected: false,
},
{
path: "/derptastic",
expected: false,
},
{
path: "/api/v2/derp",
expected: false,
},
{
path: "//",
expected: false,
},
}
for _, tc := range testcases {
tc := tc
t.Run(tc.path, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.expected, isDERPPath(tc.path))
})
}
}
+6
View File
@@ -683,6 +683,12 @@ func TestServer(t *testing.T) {
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
require.Equal(t, c.expectRedirect, resp.Header.Get("Location"))
}
// We should never redirect DERP
respDERP, err := client.Request(ctx, http.MethodGet, "/derp", nil)
require.NoError(t, err)
defer respDERP.Body.Close()
require.Equal(t, http.StatusUpgradeRequired, respDERP.StatusCode)
}
// Verify TLS