coder · deansheather · Aug 24, 2023 · Aug 22, 2023 · Aug 23, 2023 · Aug 23, 2023
diff --git 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
@@ -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
@@ -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
@@ -136,6 +136,12 @@ networking:
     # this change has been made, but new connections will still be proxied regardless.
     # (default: <unset>, 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: <unset>, type: bool)
+    forceWebSockets: false
     # URL to fetch a DERP mapping on startup. See:
     # https://tailscale.com/kb/1118/custom-derp-servers/.
     # (default: <unset>, type: string)

diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git 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
@@ -6,9 +6,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"
@@ -17,8 +21,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"
 )
@@ -115,6 +124,94 @@ 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,
+		IncludeProvisionerDaemon: true,
+	}
+	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.
+	if options.IncludeProvisionerDaemon {
+		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
@@ -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
@@ -45,16 +45,18 @@ 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,
 ) (*ServerTailnet, error) {
 	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
@@ -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
@@ -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
@@ -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
@@ -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"`