coder · deansheather · Jun 21, 2023 · Jun 9, 2023 · Jun 9, 2023 · Jun 10, 2023
diff --git a/agent/agent.go b/agent/agent.go
@@ -593,7 +593,7 @@ func (a *agent) run(ctx context.Context) error {
 	network := a.network
 	a.closeMutex.Unlock()
 	if network == nil {
-		network, err = a.createTailnet(ctx, manifest.DERPMap)
+		network, err = a.createTailnet(ctx, manifest.DERPMap, manifest.DisableDirectConnections)
 		if err != nil {
 			return xerrors.Errorf("create tailnet: %w", err)
 		}
@@ -611,8 +611,9 @@ func (a *agent) run(ctx context.Context) error {
 
 		a.startReportingConnectionStats(ctx)
 	} else {
-		// Update the DERP map!
+		// Update the DERP map and allow/disallow direct connections.
 		network.SetDERPMap(manifest.DERPMap)
+		network.SetBlockEndpoints(manifest.DisableDirectConnections)
 	}
 
 	a.logger.Debug(ctx, "running tailnet connection coordinator")
@@ -637,12 +638,13 @@ func (a *agent) trackConnGoroutine(fn func()) error {
 	return nil
 }
 
-func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (_ *tailnet.Conn, err error) {
+func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap, disableDirectConnections bool) (_ *tailnet.Conn, err error) {
 	network, err := tailnet.NewConn(&tailnet.Options{
-		Addresses:  []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
-		DERPMap:    derpMap,
-		Logger:     a.logger.Named("tailnet"),
-		ListenPort: a.tailnetListenPort,
+		Addresses:      []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
+		DERPMap:        derpMap,
+		Logger:         a.logger.Named("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
@@ -25,10 +25,14 @@ func TestNetcheck(t *testing.T) {
 
 	clitest.StartWithWaiter(t, inv).RequireSuccess()
 
+	b := out.Bytes()
+	t.Log(string(b))
 	var report healthcheck.DERPReport
-	require.NoError(t, json.Unmarshal(out.Bytes(), &report))
+	require.NoError(t, json.Unmarshal(b, &report))
 
 	assert.True(t, report.Healthy)
 	require.Len(t, report.Regions, 1)
-	require.Len(t, report.Regions[1].NodeReports, 1)
+	for _, v := range report.Regions {
+		require.Len(t, v.NodeReports, len(v.Region.Nodes))
+	}
 }
diff --git a/cli/server.go b/cli/server.go
@@ -413,6 +413,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 			derpMap, err := tailnet.NewDERPMap(
 				ctx, defaultRegion, cfg.DERP.Server.STUNAddresses,
 				cfg.DERP.Config.URL.String(), cfg.DERP.Config.Path.String(),
+				cfg.DERP.Config.BlockDirect.Value(),
 			)
 			if err != nil {
 				return xerrors.Errorf("create derp map: %w", err)

diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -150,6 +150,14 @@ between workspaces and users are peer-to-peer. However, when Coder cannot
 establish a peer to peer connection, Coder uses a distributed relay network
 backed by Tailscale and WireGuard.
 
+      --block-direct-connections bool, $CODER_BLOCK_DIRECT
+          Block peer-to-peer (aka. direct) workspace connections. All workspace
+          connections from the CLI will be proxied through Coder (or custom
+          configured DERP servers) and will never be peer-to-peer when enabled.
+          Workspaces may still reach out to STUN servers to get their address
+          until they are restarted after this change has been made, but new
+          connections will still be proxied regardless.
+
       --derp-config-path string, $CODER_DERP_CONFIG_PATH
           Path to read a DERP mapping from. See:
           https://tailscale.com/kb/1118/custom-derp-servers/.

diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden
@@ -117,6 +117,13 @@ networking:
     # for high availability.
     # (default: <unset>, type: url)
     relayURL:
+    # Block peer-to-peer (aka. direct) workspace connections. All workspace
+    # connections from the CLI will be proxied through Coder (or custom configured
+    # DERP servers) and will never be peer-to-peer when enabled. Workspaces may still
+    # reach out to STUN servers to get their address until they are restarted after
+    # this change has been made, but new connections will still be proxied regardless.
+    # (default: <unset>, type: bool)
+    blockDirect: 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/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -293,6 +293,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 	}
 
 	stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
+	stunAddr.IP = net.ParseIP("127.0.0.1")
 	t.Cleanup(stunCleanup)
 
 	derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug)))
@@ -310,6 +311,29 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 		require.NoError(t, err)
 	}
 
+	region := &tailcfg.DERPRegion{
+		EmbeddedRelay: true,
+		RegionID:      int(options.DeploymentValues.DERP.Server.RegionID.Value()),
+		RegionCode:    options.DeploymentValues.DERP.Server.RegionCode.String(),
+		RegionName:    options.DeploymentValues.DERP.Server.RegionName.String(),
+		Nodes: []*tailcfg.DERPNode{{
+			Name:     fmt.Sprintf("%db", options.DeploymentValues.DERP.Server.RegionID),
+			RegionID: int(options.DeploymentValues.DERP.Server.RegionID.Value()),
+			IPv4:     "127.0.0.1",
+			DERPPort: derpPort,
+			// STUN port is added as a separate node by tailnet.NewDERPMap() if
+			// direct connections are enabled.
+			STUNPort:         -1,
+			InsecureForTests: true,
+			ForceHTTP:        options.TLSCertificates == nil,
+		}},
+	}
+	if !options.DeploymentValues.DERP.Server.Enable.Value() {
+		region = nil
+	}
+	derpMap, err := tailnet.NewDERPMap(ctx, region, []string{stunAddr.String()}, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value())
+	require.NoError(t, err)
+
 	return func(h http.Handler) {
 			mutex.Lock()
 			defer mutex.Unlock()
@@ -328,42 +352,24 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 			Pubsub:                         options.Pubsub,
 			GitAuthConfigs:                 options.GitAuthConfigs,
 
-			Auditor:               options.Auditor,
-			AWSCertificates:       options.AWSCertificates,
-			AzureCertificates:     options.AzureCertificates,
-			GithubOAuth2Config:    options.GithubOAuth2Config,
-			RealIPConfig:          options.RealIPConfig,
-			OIDCConfig:            options.OIDCConfig,
-			GoogleTokenValidator:  options.GoogleTokenValidator,
-			SSHKeygenAlgorithm:    options.SSHKeygenAlgorithm,
-			DERPServer:            derpServer,
-			APIRateLimit:          options.APIRateLimit,
-			LoginRateLimit:        options.LoginRateLimit,
-			FilesRateLimit:        options.FilesRateLimit,
-			Authorizer:            options.Authorizer,
-			Telemetry:             telemetry.NewNoop(),
-			TemplateScheduleStore: &templateScheduleStore,
-			TLSCertificates:       options.TLSCertificates,
-			TrialGenerator:        options.TrialGenerator,
-			DERPMap: &tailcfg.DERPMap{
-				Regions: map[int]*tailcfg.DERPRegion{
-					1: {
-						EmbeddedRelay: true,
-						RegionID:      1,
-						RegionCode:    "coder",
-						RegionName:    "Coder",
-						Nodes: []*tailcfg.DERPNode{{
-							Name:             "1a",
-							RegionID:         1,
-							IPv4:             "127.0.0.1",
-							DERPPort:         derpPort,
-							STUNPort:         stunAddr.Port,
-							InsecureForTests: true,
-							ForceHTTP:        options.TLSCertificates == nil,
-						}},
-					},
-				},
-			},
+			Auditor:                     options.Auditor,
+			AWSCertificates:             options.AWSCertificates,
+			AzureCertificates:           options.AzureCertificates,
+			GithubOAuth2Config:          options.GithubOAuth2Config,
+			RealIPConfig:                options.RealIPConfig,
+			OIDCConfig:                  options.OIDCConfig,
+			GoogleTokenValidator:        options.GoogleTokenValidator,
+			SSHKeygenAlgorithm:          options.SSHKeygenAlgorithm,
+			DERPServer:                  derpServer,
+			APIRateLimit:                options.APIRateLimit,
+			LoginRateLimit:              options.LoginRateLimit,
+			FilesRateLimit:              options.FilesRateLimit,
+			Authorizer:                  options.Authorizer,
+			Telemetry:                   telemetry.NewNoop(),
+			TemplateScheduleStore:       &templateScheduleStore,
+			TLSCertificates:             options.TLSCertificates,
+			TrialGenerator:              options.TrialGenerator,
+			DERPMap:                     derpMap,
 			MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval,
 			AgentStatsRefreshInterval:   options.AgentStatsRefreshInterval,
 			DeploymentValues:            options.DeploymentValues,

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -161,18 +161,19 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
 	}
 
 	httpapi.Write(ctx, rw, http.StatusOK, agentsdk.Manifest{
-		Apps:                  convertApps(dbApps),
-		DERPMap:               api.DERPMap,
-		GitAuthConfigs:        len(api.GitAuthConfigs),
-		EnvironmentVariables:  apiAgent.EnvironmentVariables,
-		StartupScript:         apiAgent.StartupScript,
-		Directory:             apiAgent.Directory,
-		VSCodePortProxyURI:    vscodeProxyURI,
-		MOTDFile:              workspaceAgent.MOTDFile,
-		StartupScriptTimeout:  time.Duration(apiAgent.StartupScriptTimeoutSeconds) * time.Second,
-		ShutdownScript:        apiAgent.ShutdownScript,
-		ShutdownScriptTimeout: time.Duration(apiAgent.ShutdownScriptTimeoutSeconds) * time.Second,
-		Metadata:              convertWorkspaceAgentMetadataDesc(metadata),
+		Apps:                     convertApps(dbApps),
+		DERPMap:                  api.DERPMap,
+		GitAuthConfigs:           len(api.GitAuthConfigs),
+		EnvironmentVariables:     apiAgent.EnvironmentVariables,
+		StartupScript:            apiAgent.StartupScript,
+		Directory:                apiAgent.Directory,
+		VSCodePortProxyURI:       vscodeProxyURI,
+		MOTDFile:                 workspaceAgent.MOTDFile,
+		StartupScriptTimeout:     time.Duration(apiAgent.StartupScriptTimeoutSeconds) * time.Second,
+		ShutdownScript:           apiAgent.ShutdownScript,
+		ShutdownScriptTimeout:    time.Duration(apiAgent.ShutdownScriptTimeoutSeconds) * time.Second,
+		DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
+		Metadata:                 convertWorkspaceAgentMetadataDesc(metadata),
 	})
 }
 
@@ -731,9 +732,10 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
 func (api *API) dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.WorkspaceAgentConn, error) {
 	clientConn, serverConn := net.Pipe()
 	conn, err := tailnet.NewConn(&tailnet.Options{
-		Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
-		DERPMap:   api.DERPMap,
-		Logger:    api.Logger.Named("tailnet"),
+		Addresses:      []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
+		DERPMap:        api.DERPMap,
+		Logger:         api.Logger.Named("tailnet"),
+		BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
 	})
 	if err != nil {
 		_ = clientConn.Close()
@@ -801,7 +803,8 @@ func (api *API) workspaceAgentConnection(rw http.ResponseWriter, r *http.Request
 	ctx := r.Context()
 
 	httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
-		DERPMap: api.DERPMap,
+		DERPMap:                  api.DERPMap,
+		DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
 	})
 }
 

diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go
@@ -2,6 +2,7 @@ package coderd_test
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net"
 	"net/http"
@@ -575,6 +576,82 @@ func TestWorkspaceAgentTailnet(t *testing.T) {
 	require.Equal(t, "test", strings.TrimSpace(string(output)))
 }
 
+func TestWorkspaceAgentTailnetDirectDisabled(t *testing.T) {
+	t.Parallel()
+
+	dv := coderdtest.DeploymentValues(t)
+	err := dv.DERP.Config.BlockDirect.Set("true")
+	require.NoError(t, err)
+	require.True(t, dv.DERP.Config.BlockDirect.Value())
+
+	client, daemonCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
+		DeploymentValues: dv,
+	})
+	user := coderdtest.CreateFirstUser(t, client)
+	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)
+	daemonCloser.Close()
+
+	ctx := testutil.Context(t, testutil.WaitLong)
+
+	// Verify that the manifest has DisableDirectConnections set to true.
+	agentClient := agentsdk.New(client.URL)
+	agentClient.SetSessionToken(authToken)
+	manifest, err := agentClient.Manifest(ctx)
+	require.NoError(t, err)
+	require.True(t, manifest.DisableDirectConnections)
+
+	agentCloser := agent.New(agent.Options{
+		Client: agentClient,
+		Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
+	})
+	defer agentCloser.Close()
+	resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
+	agentID := resources[0].Agents[0].ID
+
+	// Verify that the connection data has no STUN ports and
+	// DisableDirectConnections set to true.
+	res, err := client.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s/connection", agentID), nil)
+	require.NoError(t, err)
+	defer res.Body.Close()
+	require.Equal(t, http.StatusOK, res.StatusCode)
+	var connInfo codersdk.WorkspaceAgentConnectionInfo
+	err = json.NewDecoder(res.Body).Decode(&connInfo)
+	require.NoError(t, err)
+	require.True(t, connInfo.DisableDirectConnections)
+	for _, region := range connInfo.DERPMap.Regions {
+		t.Logf("region %s (%v)", region.RegionCode, region.EmbeddedRelay)
+		for _, node := range region.Nodes {
+			t.Logf("  node %s (stun %d)", node.Name, node.STUNPort)
+			require.EqualValues(t, -1, node.STUNPort)
+			// tailnet.NewDERPMap() will create nodes with "stun" in the name,
+			// but not if direct is disabled.
+			require.NotContains(t, node.Name, "stun")
+			require.False(t, node.STUNOnly)
+		}
+	}
+
+	conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{
+		Logger: slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug),
+	})
+	require.NoError(t, err)
+	defer conn.Close()
+	require.True(t, conn.BlockEndpoints())
+
+	require.True(t, conn.AwaitReachable(ctx))
+	_, p2p, _, err := conn.Ping(ctx)
+	require.NoError(t, err)
+	require.False(t, p2p)
+}
+
 func TestWorkspaceAgentListeningPorts(t *testing.T) {
 	t.Parallel()