diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go index 910ddb1637a7f..060a66870a889 100644 --- a/coderd/workspaceapps.go +++ b/coderd/workspaceapps.go @@ -39,6 +39,23 @@ const ( redirectURIQueryParam = "redirect_uri" ) +// nonCanonicalHeaders is a map from "canonical" headers to the actual header we +// should send to the app in the workspace. Some headers (such as the websocket +// upgrade headers from RFC 6455) are not canonical according to the HTTP/1 +// spec. Golang has said that they will not add custom cases for these headers, +// so we need to do it ourselves. +// +// Some apps our customers use are sensitive to the case of these headers. +// +// https://github.com/golang/go/issues/18495 +var nonCanonicalHeaders = map[string]string{ + "Sec-Websocket-Accept": "Sec-WebSocket-Accept", + "Sec-Websocket-Extensions": "Sec-WebSocket-Extensions", + "Sec-Websocket-Key": "Sec-WebSocket-Key", + "Sec-Websocket-Protocol": "Sec-WebSocket-Protocol", + "Sec-Websocket-Version": "Sec-WebSocket-Version", +} + func (api *API) appHost(rw http.ResponseWriter, r *http.Request) { host := api.AppHostname if api.AccessURL.Port() != "" { @@ -708,6 +725,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res return } defer release() + proxy.Transport = conn.HTTPTransport() // This strips the session token from a workspace app request. cookieHeaders := r.Header.Values("Cookie")[:] @@ -715,7 +733,16 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res for _, cookieHeader := range cookieHeaders { r.Header.Add("Cookie", httpapi.StripCoderCookies(cookieHeader)) } - proxy.Transport = conn.HTTPTransport() + + // Convert canonicalized headers to their non-canonicalized counterparts. + // See the comment on `nonCanonicalHeaders` for more information on why this + // is necessary. + for k, v := range r.Header { + if n, ok := nonCanonicalHeaders[k]; ok { + r.Header.Del(k) + r.Header[n] = v + } + } // end span so we don't get long lived trace data tracing.EndHTTPSpan(r, http.StatusOK, trace.SpanFromContext(ctx)) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 84454cc532302..814f486597d9e 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -1,6 +1,7 @@ package coderd_test import ( + "bufio" "context" "encoding/json" "fmt" @@ -16,6 +17,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" @@ -1024,3 +1026,195 @@ func TestAppSharing(t *testing.T) { }) }) } + +func TestWorkspaceAppsNonCanonicalHeaders(t *testing.T) { + t.Parallel() + + setupNonCanonicalHeadersTest := func(t *testing.T, customAppHost ...string) (*codersdk.Client, codersdk.CreateFirstUserResponse, codersdk.Workspace, uint16) { + // Start a TCP server that manually parses the request. Golang's HTTP + // server canonicalizes all HTTP request headers it receives, so we + // can't use it to test that we forward non-canonical headers. + // #nosec + ln, err := net.Listen("tcp", ":0") + require.NoError(t, err) + go func() { + for { + c, err := ln.Accept() + if xerrors.Is(err, net.ErrClosed) { + return + } + require.NoError(t, err) + + go func() { + s := bufio.NewScanner(c) + + // Read request line. + assert.True(t, s.Scan()) + reqLine := s.Text() + assert.True(t, strings.HasPrefix(reqLine, fmt.Sprintf("GET /?%s HTTP/1.1", proxyTestAppQuery))) + + // Read headers and discard them. We collect the + // Sec-WebSocket-Key header (with a capital S) to respond + // with. + secWebSocketKey := "(none found)" + for s.Scan() { + if s.Text() == "" { + break + } + + line := strings.TrimSpace(s.Text()) + if strings.HasPrefix(line, "Sec-WebSocket-Key: ") { + secWebSocketKey = strings.TrimPrefix(line, "Sec-WebSocket-Key: ") + } + } + + // Write response containing text/plain with the + // Sec-WebSocket-Key header. + res := fmt.Sprintf("HTTP/1.1 204 No Content\r\nSec-WebSocket-Key: %s\r\nConnection: close\r\n\r\n", secWebSocketKey) + _, err = c.Write([]byte(res)) + assert.NoError(t, err) + err = c.Close() + assert.NoError(t, err) + }() + } + }() + t.Cleanup(func() { + _ = ln.Close() + }) + tcpAddr, ok := ln.Addr().(*net.TCPAddr) + require.True(t, ok) + + appHost := proxyTestSubdomainRaw + if len(customAppHost) > 0 { + appHost = customAppHost[0] + } + + client := coderdtest.New(t, &coderdtest.Options{ + AppHostname: appHost, + IncludeProvisionerDaemon: true, + AgentStatsRefreshInterval: time.Millisecond * 100, + MetricsCacheRefreshInterval: time.Millisecond * 100, + RealIPConfig: &httpmw.RealIPConfig{ + TrustedOrigins: []*net.IPNet{{ + IP: net.ParseIP("127.0.0.1"), + Mask: net.CIDRMask(8, 32), + }}, + TrustedHeaders: []string{ + "CF-Connecting-IP", + }, + }, + }) + + user := coderdtest.CreateFirstUser(t, client) + + workspace := createWorkspaceWithApps(t, client, user.OrganizationID, appHost, uint16(tcpAddr.Port)) + + // Configure the HTTP client to not follow redirects and to route all + // requests regardless of hostname to the coderd test server. + client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + } + defaultTransport, ok := http.DefaultTransport.(*http.Transport) + require.True(t, ok) + transport := defaultTransport.Clone() + transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, client.URL.Host) + } + client.HTTPClient.Transport = transport + t.Cleanup(func() { + transport.CloseIdleConnections() + }) + + return client, user, workspace, uint16(tcpAddr.Port) + } + + t.Run("ProxyPath", func(t *testing.T) { + t.Parallel() + + client, _, workspace, _ := setupNonCanonicalHeadersTest(t) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + u, err := client.URL.Parse(fmt.Sprintf("/@me/%s/apps/%s/?%s", workspace.Name, proxyTestAppNameOwner, proxyTestAppQuery)) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) + require.NoError(t, err) + + // Use a non-canonical header name. The S in Sec-WebSocket-Key should be + // capitalized according to the websocket spec, but Golang will + // lowercase it to match the HTTP/1 spec. + // + // Setting the header on the map directly will force the header to not + // be canonicalized on the client, but it will be canonicalized on the + // server. + secWebSocketKey := "test-dean-was-here" + req.Header["Sec-WebSocket-Key"] = []string{secWebSocketKey} + + req.Header.Set(codersdk.SessionCustomHeader, client.SessionToken()) + resp, err := client.HTTPClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + // The response should be a 204 No Content with the Sec-WebSocket-Key + // header set to the value we sent. + res, err := httputil.DumpResponse(resp, true) + require.NoError(t, err) + t.Log(string(res)) + require.Equal(t, http.StatusNoContent, resp.StatusCode) + require.Equal(t, secWebSocketKey, resp.Header.Get("Sec-WebSocket-Key")) + }) + + t.Run("Subdomain", func(t *testing.T) { + t.Parallel() + + appHost := proxyTestSubdomainRaw + client, _, workspace, _ := setupNonCanonicalHeadersTest(t, appHost) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + user, err := client.User(ctx, codersdk.Me) + require.NoError(t, err) + + u := fmt.Sprintf( + "http://%s--%s--%s--%s%s?%s", + proxyTestAppNameOwner, + proxyTestAgentName, + workspace.Name, + user.Username, + strings.ReplaceAll(appHost, "*", ""), + proxyTestAppQuery, + ) + + // Re-enable the default redirect behavior. + client.HTTPClient.CheckRedirect = nil + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) + require.NoError(t, err) + + // Use a non-canonical header name. The S in Sec-WebSocket-Key should be + // capitalized according to the websocket spec, but Golang will + // lowercase it to match the HTTP/1 spec. + // + // Setting the header on the map directly will force the header to not + // be canonicalized on the client, but it will be canonicalized on the + // server. + secWebSocketKey := "test-dean-was-here" + req.Header["Sec-WebSocket-Key"] = []string{secWebSocketKey} + + req.Header.Set(codersdk.SessionCustomHeader, client.SessionToken()) + resp, err := client.HTTPClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + // The response should be a 204 No Content with the Sec-WebSocket-Key + // header set to the value we sent. + res, err := httputil.DumpResponse(resp, true) + require.NoError(t, err) + t.Log(string(res)) + require.Equal(t, http.StatusNoContent, resp.StatusCode) + require.Equal(t, secWebSocketKey, resp.Header.Get("Sec-WebSocket-Key")) + }) +}