Skip to content

Commit a28d422

Browse files
deansheatherdeansheather
authored
feat: add flag to disable all direct connections (#7936)
1 parent 96f9e61 commit a28d422

22 files changed

+369
-105
lines changed

‎agent/agent.go

+9-7
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ func (a *agent) run(ctx context.Context) error {
593593
network := a.network
594594
a.closeMutex.Unlock()
595595
if network == nil {
596-
network, err = a.createTailnet(ctx, manifest.DERPMap)
596+
network, err = a.createTailnet(ctx, manifest.DERPMap, manifest.DisableDirectConnections)
597597
if err != nil {
598598
return xerrors.Errorf("create tailnet: %w", err)
599599
}
@@ -611,8 +611,9 @@ func (a *agent) run(ctx context.Context) error {
611611

612612
a.startReportingConnectionStats(ctx)
613613
} else {
614-
// Update the DERP map!
614+
// Update the DERP map and allow/disallow direct connections.
615615
network.SetDERPMap(manifest.DERPMap)
616+
network.SetBlockEndpoints(manifest.DisableDirectConnections)
616617
}
617618

618619
a.logger.Debug(ctx, "running tailnet connection coordinator")
@@ -637,12 +638,13 @@ func (a *agent) trackConnGoroutine(fn func()) error {
637638
return nil
638639
}
639640

640-
func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (_ *tailnet.Conn, err error) {
641+
func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap, disableDirectConnections bool) (_ *tailnet.Conn, err error) {
641642
network, err := tailnet.NewConn(&tailnet.Options{
642-
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
643-
DERPMap: derpMap,
644-
Logger: a.logger.Named("tailnet"),
645-
ListenPort: a.tailnetListenPort,
643+
Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.WorkspaceAgentIP, 128)},
644+
DERPMap: derpMap,
645+
Logger: a.logger.Named("tailnet"),
646+
ListenPort: a.tailnetListenPort,
647+
BlockEndpoints: disableDirectConnections,
646648
})
647649
if err != nil {
648650
return nil, xerrors.Errorf("create tailnet: %w", err)

‎cli/netcheck_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,14 @@ func TestNetcheck(t *testing.T) {
2525

2626
clitest.StartWithWaiter(t, inv).RequireSuccess()
2727

28+
b := out.Bytes()
29+
t.Log(string(b))
2830
var report healthcheck.DERPReport
29-
require.NoError(t, json.Unmarshal(out.Bytes(), &report))
31+
require.NoError(t, json.Unmarshal(b, &report))
3032

3133
assert.True(t, report.Healthy)
3234
require.Len(t, report.Regions, 1)
33-
require.Len(t, report.Regions[1].NodeReports, 1)
35+
for _, v := range report.Regions {
36+
require.Len(t, v.NodeReports, len(v.Region.Nodes))
37+
}
3438
}

‎cli/server.go

+1
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
413413
derpMap, err := tailnet.NewDERPMap(
414414
ctx, defaultRegion, cfg.DERP.Server.STUNAddresses,
415415
cfg.DERP.Config.URL.String(), cfg.DERP.Config.Path.String(),
416+
cfg.DERP.Config.BlockDirect.Value(),
416417
)
417418
if err != nil {
418419
return xerrors.Errorf("create derp map: %w", err)

‎cli/testdata/coder_server_--help.golden

+8
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,14 @@ between workspaces and users are peer-to-peer. However, when Coder cannot
150150
establish a peer to peer connection, Coder uses a distributed relay network
151151
backed by Tailscale and WireGuard.
152152

153+
--block-direct-connections bool, $CODER_BLOCK_DIRECT
154+
Block peer-to-peer (aka. direct) workspace connections. All workspace
155+
connections from the CLI will be proxied through Coder (or custom
156+
configured DERP servers) and will never be peer-to-peer when enabled.
157+
Workspaces may still reach out to STUN servers to get their address
158+
until they are restarted after this change has been made, but new
159+
connections will still be proxied regardless.
160+
153161
--derp-config-path string, $CODER_DERP_CONFIG_PATH
154162
Path to read a DERP mapping from. See:
155163
https://tailscale.com/kb/1118/custom-derp-servers/.

‎cli/testdata/server-config.yaml.golden

+7
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,13 @@ networking:
117117
# for high availability.
118118
# (default: <unset>, type: url)
119119
relayURL:
120+
# Block peer-to-peer (aka. direct) workspace connections. All workspace
121+
# connections from the CLI will be proxied through Coder (or custom configured
122+
# DERP servers) and will never be peer-to-peer when enabled. Workspaces may still
123+
# reach out to STUN servers to get their address until they are restarted after
124+
# this change has been made, but new connections will still be proxied regardless.
125+
# (default: <unset>, type: bool)
126+
blockDirect: false
120127
# URL to fetch a DERP mapping on startup. See:
121128
# https://tailscale.com/kb/1118/custom-derp-servers/.
122129
# (default: <unset>, type: string)

‎coderd/apidoc/docs.go

+9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

+9
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderdtest/coderdtest.go

+42-36
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
293293
}
294294

295295
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
296+
stunAddr.IP = net.ParseIP("127.0.0.1")
296297
t.Cleanup(stunCleanup)
297298

298299
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
310311
require.NoError(t, err)
311312
}
312313

314+
region := &tailcfg.DERPRegion{
315+
EmbeddedRelay: true,
316+
RegionID: int(options.DeploymentValues.DERP.Server.RegionID.Value()),
317+
RegionCode: options.DeploymentValues.DERP.Server.RegionCode.String(),
318+
RegionName: options.DeploymentValues.DERP.Server.RegionName.String(),
319+
Nodes: []*tailcfg.DERPNode{{
320+
Name: fmt.Sprintf("%db", options.DeploymentValues.DERP.Server.RegionID),
321+
RegionID: int(options.DeploymentValues.DERP.Server.RegionID.Value()),
322+
IPv4: "127.0.0.1",
323+
DERPPort: derpPort,
324+
// STUN port is added as a separate node by tailnet.NewDERPMap() if
325+
// direct connections are enabled.
326+
STUNPort: -1,
327+
InsecureForTests: true,
328+
ForceHTTP: options.TLSCertificates == nil,
329+
}},
330+
}
331+
if !options.DeploymentValues.DERP.Server.Enable.Value() {
332+
region = nil
333+
}
334+
derpMap, err := tailnet.NewDERPMap(ctx, region, []string{stunAddr.String()}, "", "", options.DeploymentValues.DERP.Config.BlockDirect.Value())
335+
require.NoError(t, err)
336+
313337
return func(h http.Handler) {
314338
mutex.Lock()
315339
defer mutex.Unlock()
@@ -328,42 +352,24 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
328352
Pubsub: options.Pubsub,
329353
GitAuthConfigs: options.GitAuthConfigs,
330354

331-
Auditor: options.Auditor,
332-
AWSCertificates: options.AWSCertificates,
333-
AzureCertificates: options.AzureCertificates,
334-
GithubOAuth2Config: options.GithubOAuth2Config,
335-
RealIPConfig: options.RealIPConfig,
336-
OIDCConfig: options.OIDCConfig,
337-
GoogleTokenValidator: options.GoogleTokenValidator,
338-
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
339-
DERPServer: derpServer,
340-
APIRateLimit: options.APIRateLimit,
341-
LoginRateLimit: options.LoginRateLimit,
342-
FilesRateLimit: options.FilesRateLimit,
343-
Authorizer: options.Authorizer,
344-
Telemetry: telemetry.NewNoop(),
345-
TemplateScheduleStore: &templateScheduleStore,
346-
TLSCertificates: options.TLSCertificates,
347-
TrialGenerator: options.TrialGenerator,
348-
DERPMap: &tailcfg.DERPMap{
349-
Regions: map[int]*tailcfg.DERPRegion{
350-
1: {
351-
EmbeddedRelay: true,
352-
RegionID: 1,
353-
RegionCode: "coder",
354-
RegionName: "Coder",
355-
Nodes: []*tailcfg.DERPNode{{
356-
Name: "1a",
357-
RegionID: 1,
358-
IPv4: "127.0.0.1",
359-
DERPPort: derpPort,
360-
STUNPort: stunAddr.Port,
361-
InsecureForTests: true,
362-
ForceHTTP: options.TLSCertificates == nil,
363-
}},
364-
},
365-
},
366-
},
355+
Auditor: options.Auditor,
356+
AWSCertificates: options.AWSCertificates,
357+
AzureCertificates: options.AzureCertificates,
358+
GithubOAuth2Config: options.GithubOAuth2Config,
359+
RealIPConfig: options.RealIPConfig,
360+
OIDCConfig: options.OIDCConfig,
361+
GoogleTokenValidator: options.GoogleTokenValidator,
362+
SSHKeygenAlgorithm: options.SSHKeygenAlgorithm,
363+
DERPServer: derpServer,
364+
APIRateLimit: options.APIRateLimit,
365+
LoginRateLimit: options.LoginRateLimit,
366+
FilesRateLimit: options.FilesRateLimit,
367+
Authorizer: options.Authorizer,
368+
Telemetry: telemetry.NewNoop(),
369+
TemplateScheduleStore: &templateScheduleStore,
370+
TLSCertificates: options.TLSCertificates,
371+
TrialGenerator: options.TrialGenerator,
372+
DERPMap: derpMap,
367373
MetricsCacheRefreshInterval: options.MetricsCacheRefreshInterval,
368374
AgentStatsRefreshInterval: options.AgentStatsRefreshInterval,
369375
DeploymentValues: options.DeploymentValues,

‎coderd/workspaceagents.go

+19-16
Original file line numberDiff line numberDiff line change
@@ -161,18 +161,19 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
161161
}
162162

163163
httpapi.Write(ctx, rw, http.StatusOK, agentsdk.Manifest{
164-
Apps: convertApps(dbApps),
165-
DERPMap: api.DERPMap,
166-
GitAuthConfigs: len(api.GitAuthConfigs),
167-
EnvironmentVariables: apiAgent.EnvironmentVariables,
168-
StartupScript: apiAgent.StartupScript,
169-
Directory: apiAgent.Directory,
170-
VSCodePortProxyURI: vscodeProxyURI,
171-
MOTDFile: workspaceAgent.MOTDFile,
172-
StartupScriptTimeout: time.Duration(apiAgent.StartupScriptTimeoutSeconds) * time.Second,
173-
ShutdownScript: apiAgent.ShutdownScript,
174-
ShutdownScriptTimeout: time.Duration(apiAgent.ShutdownScriptTimeoutSeconds) * time.Second,
175-
Metadata: convertWorkspaceAgentMetadataDesc(metadata),
164+
Apps: convertApps(dbApps),
165+
DERPMap: api.DERPMap,
166+
GitAuthConfigs: len(api.GitAuthConfigs),
167+
EnvironmentVariables: apiAgent.EnvironmentVariables,
168+
StartupScript: apiAgent.StartupScript,
169+
Directory: apiAgent.Directory,
170+
VSCodePortProxyURI: vscodeProxyURI,
171+
MOTDFile: workspaceAgent.MOTDFile,
172+
StartupScriptTimeout: time.Duration(apiAgent.StartupScriptTimeoutSeconds) * time.Second,
173+
ShutdownScript: apiAgent.ShutdownScript,
174+
ShutdownScriptTimeout: time.Duration(apiAgent.ShutdownScriptTimeoutSeconds) * time.Second,
175+
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
176+
Metadata: convertWorkspaceAgentMetadataDesc(metadata),
176177
})
177178
}
178179

@@ -731,9 +732,10 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
731732
func (api *API) dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.WorkspaceAgentConn, error) {
732733
clientConn, serverConn := net.Pipe()
733734
conn, err := tailnet.NewConn(&tailnet.Options{
734-
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
735-
DERPMap: api.DERPMap,
736-
Logger: api.Logger.Named("tailnet"),
735+
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
736+
DERPMap: api.DERPMap,
737+
Logger: api.Logger.Named("tailnet"),
738+
BlockEndpoints: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
737739
})
738740
if err != nil {
739741
_ = clientConn.Close()
@@ -801,7 +803,8 @@ func (api *API) workspaceAgentConnection(rw http.ResponseWriter, r *http.Request
801803
ctx := r.Context()
802804

803805
httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentConnectionInfo{
804-
DERPMap: api.DERPMap,
806+
DERPMap: api.DERPMap,
807+
DisableDirectConnections: api.DeploymentValues.DERP.Config.BlockDirect.Value(),
805808
})
806809
}
807810

‎coderd/workspaceagents_test.go

+77
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package coderd_test
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"net"
78
"net/http"
@@ -575,6 +576,82 @@ func TestWorkspaceAgentTailnet(t *testing.T) {
575576
require.Equal(t, "test", strings.TrimSpace(string(output)))
576577
}
577578

579+
func TestWorkspaceAgentTailnetDirectDisabled(t *testing.T) {
580+
t.Parallel()
581+
582+
dv := coderdtest.DeploymentValues(t)
583+
err := dv.DERP.Config.BlockDirect.Set("true")
584+
require.NoError(t, err)
585+
require.True(t, dv.DERP.Config.BlockDirect.Value())
586+
587+
client, daemonCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
588+
DeploymentValues: dv,
589+
})
590+
user := coderdtest.CreateFirstUser(t, client)
591+
authToken := uuid.NewString()
592+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
593+
Parse: echo.ParseComplete,
594+
ProvisionPlan: echo.ProvisionComplete,
595+
ProvisionApply: echo.ProvisionApplyWithAgent(authToken),
596+
})
597+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
598+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
599+
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
600+
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
601+
daemonCloser.Close()
602+
603+
ctx := testutil.Context(t, testutil.WaitLong)
604+
605+
// Verify that the manifest has DisableDirectConnections set to true.
606+
agentClient := agentsdk.New(client.URL)
607+
agentClient.SetSessionToken(authToken)
608+
manifest, err := agentClient.Manifest(ctx)
609+
require.NoError(t, err)
610+
require.True(t, manifest.DisableDirectConnections)
611+
612+
agentCloser := agent.New(agent.Options{
613+
Client: agentClient,
614+
Logger: slogtest.Make(t, nil).Named("agent").Leveled(slog.LevelDebug),
615+
})
616+
defer agentCloser.Close()
617+
resources := coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
618+
agentID := resources[0].Agents[0].ID
619+
620+
// Verify that the connection data has no STUN ports and
621+
// DisableDirectConnections set to true.
622+
res, err := client.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaceagents/%s/connection", agentID), nil)
623+
require.NoError(t, err)
624+
defer res.Body.Close()
625+
require.Equal(t, http.StatusOK, res.StatusCode)
626+
var connInfo codersdk.WorkspaceAgentConnectionInfo
627+
err = json.NewDecoder(res.Body).Decode(&connInfo)
628+
require.NoError(t, err)
629+
require.True(t, connInfo.DisableDirectConnections)
630+
for _, region := range connInfo.DERPMap.Regions {
631+
t.Logf("region %s (%v)", region.RegionCode, region.EmbeddedRelay)
632+
for _, node := range region.Nodes {
633+
t.Logf(" node %s (stun %d)", node.Name, node.STUNPort)
634+
require.EqualValues(t, -1, node.STUNPort)
635+
// tailnet.NewDERPMap() will create nodes with "stun" in the name,
636+
// but not if direct is disabled.
637+
require.NotContains(t, node.Name, "stun")
638+
require.False(t, node.STUNOnly)
639+
}
640+
}
641+
642+
conn, err := client.DialWorkspaceAgent(ctx, resources[0].Agents[0].ID, &codersdk.DialWorkspaceAgentOptions{
643+
Logger: slogtest.Make(t, nil).Named("client").Leveled(slog.LevelDebug),
644+
})
645+
require.NoError(t, err)
646+
defer conn.Close()
647+
require.True(t, conn.BlockEndpoints())
648+
649+
require.True(t, conn.AwaitReachable(ctx))
650+
_, p2p, _, err := conn.Ping(ctx)
651+
require.NoError(t, err)
652+
require.False(t, p2p)
653+
}
654+
578655
func TestWorkspaceAgentListeningPorts(t *testing.T) {
579656
t.Parallel()
580657

0 commit comments

Comments
 (0)