diff --git a/agent/agent.go b/agent/agent.go index 7622820385bd8..532e7e5a88392 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -678,7 +678,7 @@ func (a *agent) run(ctx context.Context) error { network := a.network a.closeMutex.Unlock() if network == nil { - network, err = a.createTailnet(ctx, manifest.AgentID, manifest.DERPMap, manifest.DisableDirectConnections) + network, err = a.createTailnet(ctx, manifest.AgentID, manifest.DERPMap, manifest.DERPForceWebSockets, manifest.DisableDirectConnections) if err != nil { return xerrors.Errorf("create tailnet: %w", err) } @@ -701,8 +701,10 @@ func (a *agent) run(ctx context.Context) error { if err != nil { a.logger.Error(ctx, "update tailnet addresses", slog.Error(err)) } - // Update the DERP map and allow/disallow direct connections. + // Update the DERP map, force WebSocket setting and allow/disallow + // direct connections. network.SetDERPMap(manifest.DERPMap) + network.SetDERPForceWebSockets(manifest.DERPForceWebSockets) network.SetBlockEndpoints(manifest.DisableDirectConnections) } @@ -756,14 +758,15 @@ func (a *agent) trackConnGoroutine(fn func()) error { return nil } -func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *tailcfg.DERPMap, disableDirectConnections bool) (_ *tailnet.Conn, err error) { +func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *tailcfg.DERPMap, derpForceWebSockets, disableDirectConnections bool) (_ *tailnet.Conn, err error) { network, err := tailnet.NewConn(&tailnet.Options{ - ID: agentID, - Addresses: a.wireguardAddresses(agentID), - DERPMap: derpMap, - Logger: a.logger.Named("net.tailnet"), - ListenPort: a.tailnetListenPort, - BlockEndpoints: disableDirectConnections, + ID: agentID, + Addresses: a.wireguardAddresses(agentID), + DERPMap: derpMap, + DERPForceWebSockets: derpForceWebSockets, + Logger: a.logger.Named("net.tailnet"), + ListenPort: a.tailnetListenPort, + BlockEndpoints: disableDirectConnections, }) if err != nil { return nil, xerrors.Errorf("create tailnet: %w", err) diff --git a/cli/netcheck_test.go b/cli/netcheck_test.go index 75fda30ff870a..aff65d565bd27 100644 --- a/cli/netcheck_test.go +++ b/cli/netcheck_test.go @@ -31,7 +31,7 @@ func TestNetcheck(t *testing.T) { require.NoError(t, json.Unmarshal(b, &report)) assert.True(t, report.Healthy) - require.Len(t, report.Regions, 1+5) // 1 built-in region + 5 STUN regions by default + require.Len(t, report.Regions, 1+1) // 1 built-in region + 1 test-managed STUN region for _, v := range report.Regions { require.Len(t, v.NodeReports, len(v.Region.Nodes)) } diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 46fddeed2d6cc..d3a5d74bcddbe 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -172,6 +172,13 @@ backed by Tailscale and WireGuard. URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/. + --derp-force-websockets bool, $CODER_DERP_FORCE_WEBSOCKETS + Force clients and agents to always use WebSocket to connect to DERP + relay servers. By default, DERP uses `Upgrade: derp`, which may cause + issues with some reverse proxies. Clients may automatically fallback + to WebSocket if they detect an issue with `Upgrade: derp`, but this + does not work in all situations. + --derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true) Whether to enable or disable the embedded DERP relay server. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 381920662cf05..166e9f02d9465 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -136,6 +136,12 @@ networking: # this change has been made, but new connections will still be proxied regardless. # (default: , type: bool) blockDirect: false + # Force clients and agents to always use WebSocket to connect to DERP relay + # servers. By default, DERP uses `Upgrade: derp`, which may cause issues with some + # reverse proxies. Clients may automatically fallback to WebSocket if they detect + # an issue with `Upgrade: derp`, but this does not work in all situations. + # (default: , type: bool) + forceWebSockets: false # URL to fetch a DERP mapping on startup. See: # https://tailscale.com/kb/1118/custom-derp-servers/. # (default: , type: string) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 951e1ed04d254..0af41e03271ea 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6416,6 +6416,9 @@ const docTemplate = `{ "$ref": "#/definitions/codersdk.WorkspaceApp" } }, + "derp_force_websockets": { + "type": "boolean" + }, "derpmap": { "$ref": "#/definitions/tailcfg.DERPMap" }, @@ -7781,6 +7784,9 @@ const docTemplate = `{ "block_direct": { "type": "boolean" }, + "force_websockets": { + "type": "boolean" + }, "path": { "type": "string" }, @@ -10710,6 +10716,9 @@ const docTemplate = `{ "codersdk.WorkspaceAgentConnectionInfo": { "type": "object", "properties": { + "derp_force_websockets": { + "type": "boolean" + }, "derp_map": { "$ref": "#/definitions/tailcfg.DERPMap" }, @@ -11973,6 +11982,9 @@ const docTemplate = `{ "app_security_key": { "type": "string" }, + "derp_force_websockets": { + "type": "boolean" + }, "derp_map": { "$ref": "#/definitions/tailcfg.DERPMap" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a0cea25ad07ed..008534328fd70 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5656,6 +5656,9 @@ "$ref": "#/definitions/codersdk.WorkspaceApp" } }, + "derp_force_websockets": { + "type": "boolean" + }, "derpmap": { "$ref": "#/definitions/tailcfg.DERPMap" }, @@ -6936,6 +6939,9 @@ "block_direct": { "type": "boolean" }, + "force_websockets": { + "type": "boolean" + }, "path": { "type": "string" }, @@ -9712,6 +9718,9 @@ "codersdk.WorkspaceAgentConnectionInfo": { "type": "object", "properties": { + "derp_force_websockets": { + "type": "boolean" + }, "derp_map": { "$ref": "#/definitions/tailcfg.DERPMap" }, @@ -10934,6 +10943,9 @@ "app_security_key": { "type": "string" }, + "derp_force_websockets": { + "type": "boolean" + }, "derp_map": { "$ref": "#/definitions/tailcfg.DERPMap" }, diff --git a/coderd/coderd.go b/coderd/coderd.go index 5f3bbab1a0360..8fbad62794ab5 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -405,6 +405,7 @@ func New(options *Options) *API { options.Logger, options.DERPServer, api.DERPMap, + options.DeploymentValues.DERP.Config.ForceWebSockets.Value(), func(context.Context) (tailnet.MultiAgentConn, error) { return (*api.TailnetCoordinator.Load()).ServeMultiAgent(uuid.New()), nil }, diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index 1805b15c959ce..1924c68439508 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -7,9 +7,13 @@ import ( "net/http" "net/netip" "strconv" + "strings" "sync" + "sync/atomic" "testing" + "github.com/davecgh/go-spew/spew" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/goleak" @@ -18,8 +22,13 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/buildinfo" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/codersdk/agentsdk" + "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/testutil" ) @@ -119,6 +128,91 @@ func TestDERP(t *testing.T) { w2.Close() } +func TestDERPForceWebSockets(t *testing.T) { + t.Parallel() + + dv := coderdtest.DeploymentValues(t) + dv.DERP.Config.ForceWebSockets = true + dv.DERP.Config.BlockDirect = true // to ensure the test always uses DERP + + // Manually create a server so we can influence the HTTP handler. + options := &coderdtest.Options{ + DeploymentValues: dv, + } + setHandler, cancelFunc, serverURL, newOptions := coderdtest.NewOptions(t, options) + coderAPI := coderd.New(newOptions) + t.Cleanup(func() { + cancelFunc() + _ = coderAPI.Close() + }) + + // Set the HTTP handler to a custom one that ensures all /derp calls are + // WebSockets and not `Upgrade: derp`. + var upgradeCount int64 + setHandler(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/derp") { + up := r.Header.Get("Upgrade") + if up != "" && up != "websocket" { + t.Errorf("expected Upgrade: websocket, got %q", up) + } else { + atomic.AddInt64(&upgradeCount, 1) + } + } + + coderAPI.RootHandler.ServeHTTP(rw, r) + })) + + // Start a provisioner daemon. + provisionerCloser := coderdtest.NewProvisionerDaemon(t, coderAPI) + t.Cleanup(func() { + _ = provisionerCloser.Close() + }) + + client := codersdk.New(serverURL) + t.Cleanup(func() { + client.HTTPClient.CloseIdleConnections() + }) + user := coderdtest.CreateFirstUser(t, client) + + gen, err := client.WorkspaceAgentConnectionInfoGeneric(context.Background()) + require.NoError(t, err) + t.Log(spew.Sdump(gen)) + + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + agentClient := agentsdk.New(client.URL) + agentClient.SetSessionToken(authToken) + agentCloser := agent.New(agent.Options{ + Client: agentClient, + Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug), + }) + defer func() { + _ = agentCloser.Close() + }() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, nil) + require.NoError(t, err) + defer func() { + _ = conn.Close() + }() + conn.AwaitReachable(ctx) + + require.GreaterOrEqual(t, atomic.LoadInt64(&upgradeCount), int64(1), "expected at least one /derp call") +} + func TestDERPLatencyCheck(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index bc2cb5e5925a0..b915b9ffbd3da 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -326,7 +326,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can stunAddresses []string dvStunAddresses = options.DeploymentValues.DERP.Server.STUNAddresses.Value() ) - if len(dvStunAddresses) == 0 || (len(dvStunAddresses) == 1 && dvStunAddresses[0] == "stun.l.google.com:19302") { + if len(dvStunAddresses) == 0 || dvStunAddresses[0] == "stun.l.google.com:19302" { stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{}) stunAddr.IP = net.ParseIP("127.0.0.1") t.Cleanup(stunCleanup) diff --git a/coderd/tailnet.go b/coderd/tailnet.go index 048fd9752f4e0..ca2a86d27f71e 100644 --- a/coderd/tailnet.go +++ b/coderd/tailnet.go @@ -45,6 +45,7 @@ func NewServerTailnet( logger slog.Logger, derpServer *derp.Server, derpMapFn func() *tailcfg.DERPMap, + derpForceWebSockets bool, getMultiAgent func(context.Context) (tailnet.MultiAgentConn, error), cache *wsconncache.Cache, traceProvider trace.TracerProvider, @@ -52,9 +53,10 @@ func NewServerTailnet( logger = logger.Named("servertailnet") originalDerpMap := derpMapFn() conn, err := tailnet.NewConn(&tailnet.Options{ - Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, - DERPMap: originalDerpMap, - Logger: logger, + Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, + DERPMap: originalDerpMap, + DERPForceWebSockets: derpForceWebSockets, + Logger: logger, }) if err != nil { return nil, xerrors.Errorf("create tailnet conn: %w", err) diff --git a/coderd/tailnet_test.go b/coderd/tailnet_test.go index 634a715abfbd7..2a0b0dfdbae70 100644 --- a/coderd/tailnet_test.go +++ b/coderd/tailnet_test.go @@ -232,6 +232,7 @@ func setupAgent(t *testing.T, agentAddresses []netip.Prefix) (uuid.UUID, agent.A logger, derpServer, func() *tailcfg.DERPMap { return manifest.DERPMap }, + false, func(context.Context) (tailnet.MultiAgentConn, error) { return coord.ServeMultiAgent(uuid.New()), nil }, cache, trace.NewNoopTracerProvider(), diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 2839ae4b83cce..3f90abb3a4b9b 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -167,6 +167,7 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request) AgentID: apiAgent.ID, Apps: convertApps(dbApps), DERPMap: api.DERPMap(), + DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(), GitAuthConfigs: len(api.GitAuthConfigs), EnvironmentVariables: apiAgent.EnvironmentVariables, StartupScript: apiAgent.StartupScript, @@ -733,10 +734,11 @@ func (api *API) _dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.Workspa derpMap := api.DERPMap() conn, err := tailnet.NewConn(&tailnet.Options{ - Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, - DERPMap: api.DERPMap(), - Logger: api.Logger.Named("net.tailnet"), - BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(), + Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, + DERPMap: api.DERPMap(), + DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(), + Logger: api.Logger.Named("net.tailnet"), + BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(), }) if err != nil { _ = clientConn.Close() @@ -831,6 +833,7 @@ func (api *API) workspaceAgentConnection(rw http.ResponseWriter, r *http.Request httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{ DERPMap: api.DERPMap(), + DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(), DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(), }) } @@ -851,6 +854,7 @@ func (api *API) workspaceAgentConnectionGeneric(rw http.ResponseWriter, r *http. httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{ DERPMap: api.DERPMap(), + DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(), DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(), }) } diff --git a/coderd/wsconncache/wsconncache_test.go b/coderd/wsconncache/wsconncache_test.go index f06f836bd3ab7..68e41b17517fa 100644 --- a/coderd/wsconncache/wsconncache_test.go +++ b/coderd/wsconncache/wsconncache_test.go @@ -179,9 +179,10 @@ func setupAgent(t *testing.T, manifest agentsdk.Manifest, ptyTimeout time.Durati _ = closer.Close() }) conn, err := tailnet.NewConn(&tailnet.Options{ - Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, - DERPMap: manifest.DERPMap, - Logger: slogtest.Make(t, nil).Named("tailnet").Leveled(slog.LevelDebug), + Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)}, + DERPMap: manifest.DERPMap, + DERPForceWebSockets: manifest.DERPForceWebSockets, + Logger: slogtest.Make(t, nil).Named("tailnet").Leveled(slog.LevelDebug), }) require.NoError(t, err) clientConn, serverConn := net.Pipe() diff --git a/codersdk/agentsdk/agentsdk.go b/codersdk/agentsdk/agentsdk.go index b81aacc96bc64..fb1b2f497410b 100644 --- a/codersdk/agentsdk/agentsdk.go +++ b/codersdk/agentsdk/agentsdk.go @@ -89,6 +89,7 @@ type Manifest struct { VSCodePortProxyURI string `json:"vscode_port_proxy_uri"` Apps []codersdk.WorkspaceApp `json:"apps"` DERPMap *tailcfg.DERPMap `json:"derpmap"` + DERPForceWebSockets bool `json:"derp_force_websockets"` EnvironmentVariables map[string]string `json:"environment_variables"` StartupScript string `json:"startup_script"` StartupScriptTimeout time.Duration `json:"startup_script_timeout"` diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 708bc9e899784..a8356b6816554 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -229,9 +229,10 @@ type DERPServerConfig struct { } type DERPConfig struct { - BlockDirect clibase.Bool `json:"block_direct" typescript:",notnull"` - URL clibase.String `json:"url" typescript:",notnull"` - Path clibase.String `json:"path" typescript:",notnull"` + BlockDirect clibase.Bool `json:"block_direct" typescript:",notnull"` + ForceWebSockets clibase.Bool `json:"force_websockets" typescript:",notnull"` + URL clibase.String `json:"url" typescript:",notnull"` + Path clibase.String `json:"path" typescript:",notnull"` } type PrometheusConfig struct { @@ -797,6 +798,15 @@ when required by your organization's security policy.`, Group: &deploymentGroupNetworkingDERP, YAML: "blockDirect", }, + { + Name: "DERP Force WebSockets", + Description: "Force clients and agents to always use WebSocket to connect to DERP relay servers. By default, DERP uses `Upgrade: derp`, which may cause issues with some reverse proxies. Clients may automatically fallback to WebSocket if they detect an issue with `Upgrade: derp`, but this does not work in all situations.", + Flag: "derp-force-websockets", + Env: "CODER_DERP_FORCE_WEBSOCKETS", + Value: &c.DERP.Config.ForceWebSockets, + Group: &deploymentGroupNetworkingDERP, + YAML: "forceWebSockets", + }, { Name: "DERP Config URL", Description: "URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/.", diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 7eb20a3d5d9f9..bbb6c373c6984 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -186,6 +186,7 @@ type DERPRegion struct { // @typescript-ignore WorkspaceAgentConnectionInfo type WorkspaceAgentConnectionInfo struct { DERPMap *tailcfg.DERPMap `json:"derp_map"` + DERPForceWebSockets bool `json:"derp_force_websockets"` DisableDirectConnections bool `json:"disable_direct_connections"` } @@ -247,11 +248,12 @@ func (c *Client) DialWorkspaceAgent(ctx context.Context, agentID uuid.UUID, opti header = headerTransport.Header() } conn, err := tailnet.NewConn(&tailnet.Options{ - Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)}, - DERPMap: connInfo.DERPMap, - DERPHeader: &header, - Logger: options.Logger, - BlockEndpoints: c.DisableDirectConnections || options.BlockEndpoints, + Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)}, + DERPMap: connInfo.DERPMap, + DERPHeader: &header, + DERPForceWebSockets: connInfo.DERPForceWebSockets, + Logger: options.Logger, + BlockEndpoints: c.DisableDirectConnections || options.BlockEndpoints, }) if err != nil { return nil, xerrors.Errorf("create tailnet: %w", err) diff --git a/docs/api/agents.md b/docs/api/agents.md index 73b4abf8ec84b..e1eaafa060411 100644 --- a/docs/api/agents.md +++ b/docs/api/agents.md @@ -392,6 +392,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/me/manifest \ "url": "string" } ], + "derp_force_websockets": true, "derpmap": { "homeParams": { "regionScore": { @@ -743,6 +744,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con ```json { + "derp_force_websockets": true, "derp_map": { "homeParams": { "regionScore": { diff --git a/docs/api/general.md b/docs/api/general.md index 170d52a0f3260..604a2993723e5 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -168,6 +168,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "derp": { "config": { "block_direct": true, + "force_websockets": true, "path": "string", "url": "string" }, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index b61907f0cd906..3cb49b84bf833 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -201,6 +201,7 @@ "url": "string" } ], + "derp_force_websockets": true, "derpmap": { "homeParams": { "regionScore": { @@ -291,6 +292,7 @@ | ---------------------------- | ------------------------------------------------------------------------------------------------- | -------- | ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | | `agent_id` | string | false | | | | `apps` | array of [codersdk.WorkspaceApp](#codersdkworkspaceapp) | false | | | +| `derp_force_websockets` | boolean | false | | | | `derpmap` | [tailcfg.DERPMap](#tailcfgderpmap) | false | | | | `directory` | string | false | | | | `disable_direct_connections` | boolean | false | | | @@ -1812,6 +1814,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in { "config": { "block_direct": true, + "force_websockets": true, "path": "string", "url": "string" }, @@ -1850,6 +1853,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ```json { "block_direct": true, + "force_websockets": true, "path": "string", "url": "string" } @@ -1857,11 +1861,12 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| -------------- | ------- | -------- | ------------ | ----------- | -| `block_direct` | boolean | false | | | -| `path` | string | false | | | -| `url` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ------------------ | ------- | -------- | ------------ | ----------- | +| `block_direct` | boolean | false | | | +| `force_websockets` | boolean | false | | | +| `path` | string | false | | | +| `url` | string | false | | | ## codersdk.DERPRegion @@ -1985,6 +1990,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "derp": { "config": { "block_direct": true, + "force_websockets": true, "path": "string", "url": "string" }, @@ -2347,6 +2353,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "derp": { "config": { "block_direct": true, + "force_websockets": true, "path": "string", "url": "string" }, @@ -5642,6 +5649,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { + "derp_force_websockets": true, "derp_map": { "homeParams": { "regionScore": { @@ -5709,6 +5717,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | Name | Type | Required | Restrictions | Description | | ---------------------------- | ---------------------------------- | -------- | ------------ | ----------- | +| `derp_force_websockets` | boolean | false | | | | `derp_map` | [tailcfg.DERPMap](#tailcfgderpmap) | false | | | | `disable_direct_connections` | boolean | false | | | @@ -7722,6 +7731,7 @@ _None_ ```json { "app_security_key": "string", + "derp_force_websockets": true, "derp_map": { "homeParams": { "regionScore": { @@ -7799,13 +7809,14 @@ _None_ ### Properties -| Name | Type | Required | Restrictions | Description | -| ------------------ | --------------------------------------------- | -------- | ------------ | -------------------------------------------------------------------------------------- | -| `app_security_key` | string | false | | | -| `derp_map` | [tailcfg.DERPMap](#tailcfgderpmap) | false | | | -| `derp_mesh_key` | string | false | | | -| `derp_region_id` | integer | false | | | -| `sibling_replicas` | array of [codersdk.Replica](#codersdkreplica) | false | | Sibling replicas is a list of all other replicas of the proxy that have not timed out. | +| Name | Type | Required | Restrictions | Description | +| ----------------------- | --------------------------------------------- | -------- | ------------ | -------------------------------------------------------------------------------------- | +| `app_security_key` | string | false | | | +| `derp_force_websockets` | boolean | false | | | +| `derp_map` | [tailcfg.DERPMap](#tailcfgderpmap) | false | | | +| `derp_mesh_key` | string | false | | | +| `derp_region_id` | integer | false | | | +| `sibling_replicas` | array of [codersdk.Replica](#codersdkreplica) | false | | Sibling replicas is a list of all other replicas of the proxy that have not timed out. | ## wsproxysdk.ReportAppStatsRequest diff --git a/docs/cli/server.md b/docs/cli/server.md index a2f29c39dca04..49ba37d7a4236 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -118,6 +118,16 @@ Path to read a DERP mapping from. See: https://tailscale.com/kb/1118/custom-derp URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/. +### --derp-force-websockets + +| | | +| ----------- | -------------------------------------------- | +| Type | bool | +| Environment | $CODER_DERP_FORCE_WEBSOCKETS | +| YAML | networking.derp.forceWebSockets | + +Force clients and agents to always use WebSocket to connect to DERP relay servers. By default, DERP uses `Upgrade: derp`, which may cause issues with some reverse proxies. Clients may automatically fallback to WebSocket if they detect an issue with `Upgrade: derp`, but this does not work in all situations. + ### --derp-server-enable | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 46fddeed2d6cc..d3a5d74bcddbe 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -172,6 +172,13 @@ backed by Tailscale and WireGuard. URL to fetch a DERP mapping on startup. See: https://tailscale.com/kb/1118/custom-derp-servers/. + --derp-force-websockets bool, $CODER_DERP_FORCE_WEBSOCKETS + Force clients and agents to always use WebSocket to connect to DERP + relay servers. By default, DERP uses `Upgrade: derp`, which may cause + issues with some reverse proxies. Clients may automatically fallback + to WebSocket if they detect an issue with `Upgrade: derp`, but this + does not work in all situations. + --derp-server-enable bool, $CODER_DERP_SERVER_ENABLE (default: true) Whether to enable or disable the embedded DERP relay server. diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index faf665ffb8e5d..22ab937e7f3ce 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -717,11 +717,12 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) // aReq.New = updatedProxy httpapi.Write(ctx, rw, http.StatusCreated, wsproxysdk.RegisterWorkspaceProxyResponse{ - AppSecurityKey: api.AppSecurityKey.String(), - DERPMeshKey: api.DERPServer.MeshKey(), - DERPRegionID: regionID, - DERPMap: api.AGPL.DERPMap(), - SiblingReplicas: siblingsRes, + AppSecurityKey: api.AppSecurityKey.String(), + DERPMeshKey: api.DERPServer.MeshKey(), + DERPRegionID: regionID, + DERPMap: api.AGPL.DERPMap(), + DERPForceWebSockets: api.DeploymentValues.DERP.Config.ForceWebSockets.Value(), + SiblingReplicas: siblingsRes, }) go api.forceWorkspaceProxyHealthUpdate(api.ctx) diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index 9b3deab5624a2..b0194d69d3f26 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -11,6 +11,7 @@ import ( "reflect" "regexp" "strings" + "sync/atomic" "time" "github.com/go-chi/chi/v5" @@ -121,7 +122,7 @@ type Server struct { // DERP derpMesh *derpmesh.Mesh - latestDERPMap *tailcfg.DERPMap + latestDERPMap atomic.Pointer[tailcfg.DERPMap] // Used for graceful shutdown. Required for the dialer. ctx context.Context @@ -247,8 +248,9 @@ func New(ctx context.Context, opts *Options) (*Server, error) { s.Logger, nil, func() *tailcfg.DERPMap { - return s.latestDERPMap + return s.latestDERPMap.Load() }, + regResp.DERPForceWebSockets, s.DialCoordinator, wsconncache.New(s.DialWorkspaceAgent, 0), s.TracerProvider, @@ -455,7 +457,7 @@ func (s *Server) handleRegister(_ context.Context, res wsproxysdk.RegisterWorksp } s.derpMesh.SetAddresses(addresses, false) - s.latestDERPMap = res.DERPMap + s.latestDERPMap.Store(res.DERPMap) return nil } diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index 711614908d834..74c381c2d8b4a 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -207,10 +207,11 @@ type RegisterWorkspaceProxyRequest struct { } type RegisterWorkspaceProxyResponse struct { - AppSecurityKey string `json:"app_security_key"` - DERPMeshKey string `json:"derp_mesh_key"` - DERPRegionID int32 `json:"derp_region_id"` - DERPMap *tailcfg.DERPMap `json:"derp_map"` + AppSecurityKey string `json:"app_security_key"` + DERPMeshKey string `json:"derp_mesh_key"` + DERPRegionID int32 `json:"derp_region_id"` + DERPMap *tailcfg.DERPMap `json:"derp_map"` + DERPForceWebSockets bool `json:"derp_force_websockets"` // SiblingReplicas is a list of all other replicas of the proxy that have // not timed out. SiblingReplicas []codersdk.Replica `json:"sibling_replicas"` diff --git a/go.mod b/go.mod index 611a1713ae1d6..86b32dfe3c2a0 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ replace github.com/dlclark/regexp2 => github.com/dlclark/regexp2 v1.7.0 // There are a few minor changes we make to Tailscale that we're slowly upstreaming. Compare here: // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main -replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230815060514-ebed8c967bd2 +replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230824143504-4a17d5b8a684 // This is replaced to include a fix that causes a deadlock when closing the // wireguard network. diff --git a/go.sum b/go.sum index ccdb57b437e56..3ffd6f5ecf076 100644 --- a/go.sum +++ b/go.sum @@ -218,8 +218,8 @@ github.com/coder/retry v1.4.0 h1:g0fojHFxcdgM3sBULqgjFDxw1UIvaCqk4ngUDu0EWag= github.com/coder/retry v1.4.0/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= github.com/coder/ssh v0.0.0-20230621095435-9a7e23486f1c h1:TI7TzdFI0UvQmwgyQhtI1HeyYNRxAQpr8Tw/rjT8VSA= github.com/coder/ssh v0.0.0-20230621095435-9a7e23486f1c/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= -github.com/coder/tailscale v1.1.1-0.20230815060514-ebed8c967bd2 h1:kHuTT70/yda7hdB8vi87gmgp5SgFf+oFT9d9aQ8aeXw= -github.com/coder/tailscale v1.1.1-0.20230815060514-ebed8c967bd2/go.mod h1:L8tPrwSi31RAMEMV8rjb0vYTGs7rXt8rAHbqY/p41j4= +github.com/coder/tailscale v1.1.1-0.20230824143504-4a17d5b8a684 h1:U1Nn5eL1gid6mOvu+L0u6t0gIB7uLV/7CFTOQNwsu6A= +github.com/coder/tailscale v1.1.1-0.20230824143504-4a17d5b8a684/go.mod h1:L8tPrwSi31RAMEMV8rjb0vYTGs7rXt8rAHbqY/p41j4= github.com/coder/terraform-provider-coder v0.11.1 h1:1sXcHfQrX8XhmLbtKxBED2lZ5jk3/ezBtaw6uVhpJZ4= github.com/coder/terraform-provider-coder v0.11.1/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY= github.com/coder/wgtunnel v0.1.5 h1:WP3sCj/3iJ34eKvpMQEp1oJHvm24RYh0NHbj1kfUKfs= diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ebe8fb61218e2..39f5401d5ff29 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -305,6 +305,7 @@ export interface DERP { // From codersdk/deployment.go export interface DERPConfig { readonly block_direct: boolean + readonly force_websockets: boolean readonly url: string readonly path: string } diff --git a/tailnet/conn.go b/tailnet/conn.go index 62865ea4db926..2a453a8a10340 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -88,6 +88,11 @@ type Options struct { Addresses []netip.Prefix DERPMap *tailcfg.DERPMap DERPHeader *http.Header + // DERPForceWebSockets determines whether websockets is always used for DERP + // connections, rather than trying `Upgrade: derp` first and potentially + // falling back. This is useful for misbehaving proxies that prevent + // fallback due to odd behavior, like Azure App Proxy. + DERPForceWebSockets bool // BlockEndpoints specifies whether P2P endpoints are blocked. // If so, only DERPs can establish connections. @@ -214,6 +219,7 @@ func NewConn(options *Options) (conn *Conn, err error) { sys.Set(wireguardEngine) magicConn := sys.MagicSock.Get() + magicConn.SetDERPForceWebsockets(options.DERPForceWebSockets) if options.DERPHeader != nil { magicConn.SetDERPHeader(options.DERPHeader.Clone()) } @@ -277,6 +283,7 @@ func NewConn(options *Options) (conn *Conn, err error) { dialContext, dialCancel := context.WithCancel(context.Background()) server := &Conn{ blockEndpoints: options.BlockEndpoints, + derpForceWebSockets: options.DERPForceWebSockets, dialContext: dialContext, dialCancel: dialCancel, closed: make(chan struct{}), @@ -285,7 +292,7 @@ func NewConn(options *Options) (conn *Conn, err error) { dialer: dialer, listeners: map[listenKey]*listener{}, peerMap: map[tailcfg.NodeID]*tailcfg.Node{}, - lastDERPForcedWebsockets: map[int]string{}, + lastDERPForcedWebSockets: map[int]string{}, tunDevice: sys.Tun.Get(), netMap: netMap, netStack: netStack, @@ -338,11 +345,11 @@ func NewConn(options *Options) (conn *Conn, err error) { magicConn.SetDERPForcedWebsocketCallback(func(region int, reason string) { server.logger.Debug(context.Background(), "derp forced websocket", slog.F("region", region), slog.F("reason", reason)) server.lastMutex.Lock() - if server.lastDERPForcedWebsockets[region] == reason { + if server.lastDERPForcedWebSockets[region] == reason { server.lastMutex.Unlock() return } - server.lastDERPForcedWebsockets[region] = reason + server.lastDERPForcedWebSockets[region] = reason server.lastMutex.Unlock() server.sendNode() }) @@ -383,12 +390,13 @@ func IPFromUUID(uid uuid.UUID) netip.Addr { // Conn is an actively listening Wireguard connection. type Conn struct { - dialContext context.Context - dialCancel context.CancelFunc - mutex sync.Mutex - closed chan struct{} - logger slog.Logger - blockEndpoints bool + dialContext context.Context + dialCancel context.CancelFunc + mutex sync.Mutex + closed chan struct{} + logger slog.Logger + blockEndpoints bool + derpForceWebSockets bool dialer *tsdial.Dialer tunDevice *tstun.Wrapper @@ -408,7 +416,7 @@ type Conn struct { // so the values must be stored for retrieval later on. lastStatus time.Time lastEndpoints []tailcfg.Endpoint - lastDERPForcedWebsockets map[int]string + lastDERPForcedWebSockets map[int]string lastNetInfo *tailcfg.NetInfo nodeCallback func(node *Node) @@ -461,6 +469,10 @@ func (c *Conn) SetDERPMap(derpMap *tailcfg.DERPMap) { c.wireguardEngine.SetNetworkMap(&netMapCopy) } +func (c *Conn) SetDERPForceWebSockets(v bool) { + c.magicConn.SetDERPForceWebsockets(v) +} + // SetBlockEndpoints sets whether or not to block P2P endpoints. This setting // will only apply to new peers. func (c *Conn) SetBlockEndpoints(blockEndpoints bool) { @@ -838,8 +850,16 @@ func (c *Conn) selfNode() *Node { if c.lastNetInfo != nil { preferredDERP = c.lastNetInfo.PreferredDERP derpLatency = c.lastNetInfo.DERPLatency - for k, v := range c.lastDERPForcedWebsockets { - derpForcedWebsocket[k] = v + + if c.derpForceWebSockets { + // We only need to store this for a single region, since this is + // mostly used for debugging purposes and doesn't actually have a + // code purpose. + derpForcedWebsocket[preferredDERP] = "DERP is configured to always fallback to WebSockets" + } else { + for k, v := range c.lastDERPForcedWebSockets { + derpForcedWebsocket[k] = v + } } }