Skip to content

Commit 07df3ab

Browse files
spikecurtisspikecurtis
committed
Only clients initiate wireguard handshakes
Signed-off-by: Spike Curtis <spike@coder.com>
1 parent caa2578 commit 07df3ab

File tree

10 files changed

+34
-17
lines changed

10 files changed

+34
-17
lines changed

‎agent/agent.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,7 @@ func (a *agent) trackConnGoroutine(fn func()) error {
702702
}
703703

704704
func (a *agent) createTailnet(ctx context.Context, agentID uuid.UUID, derpMap *tailcfg.DERPMap, disableDirectConnections bool) (_ *tailnet.Conn, err error) {
705-
network, err := tailnet.NewConn(&tailnet.Options{
705+
network, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
706706
Addresses: a.wireguardAddresses(agentID),
707707
DERPMap: derpMap,
708708
Logger: a.logger.Named("tailnet"),

‎agent/agent_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1914,7 +1914,7 @@ func setupAgent(t *testing.T, metadata agentsdk.Manifest, ptyTimeout time.Durati
19141914
t.Cleanup(func() {
19151915
_ = closer.Close()
19161916
})
1917-
conn, err := tailnet.NewConn(&tailnet.Options{
1917+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
19181918
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
19191919
DERPMap: metadata.DERPMap,
19201920
Logger: logger.Named("client"),

‎coderd/coderd_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,18 @@ func TestDERP(t *testing.T) {
6565
},
6666
},
6767
}
68+
// it's a bit arbitrary which is the client and which is the agent,
69+
// but, we need one of each because the client initiates the wireguard
70+
// connection.
6871
w1IP := tailnet.IP()
69-
w1, err := tailnet.NewConn(&tailnet.Options{
72+
w1, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
7073
Addresses: []netip.Prefix{netip.PrefixFrom(w1IP, 128)},
7174
Logger: logger.Named("w1"),
7275
DERPMap: derpMap,
7376
})
7477
require.NoError(t, err)
7578

76-
w2, err := tailnet.NewConn(&tailnet.Options{
79+
w2, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
7780
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
7881
Logger: logger.Named("w2"),
7982
DERPMap: derpMap,

‎coderd/tailnet.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func NewServerTailnet(
4545
cache *wsconncache.Cache,
4646
) (*ServerTailnet, error) {
4747
logger = logger.Named("servertailnet")
48-
conn, err := tailnet.NewConn(&tailnet.Options{
48+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
4949
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
5050
DERPMap: derpMap,
5151
Logger: logger,

‎coderd/tailnet_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ func setupAgent(t *testing.T, agentAddresses []netip.Prefix) (uuid.UUID, agent.A
160160
}, testutil.WaitShort, testutil.IntervalFast)
161161

162162
cache := wsconncache.New(func(id uuid.UUID) (*codersdk.WorkspaceAgentConn, error) {
163-
conn, err := tailnet.NewConn(&tailnet.Options{
163+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
164164
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
165165
DERPMap: manifest.DERPMap,
166166
Logger: logger.Named("client"),

‎coderd/workspaceagents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ func (api *API) workspaceAgentListeningPorts(rw http.ResponseWriter, r *http.Req
734734
// See: https://github.com/coder/coder/issues/8218
735735
func (api *API) _dialWorkspaceAgentTailnet(agentID uuid.UUID) (*codersdk.WorkspaceAgentConn, error) {
736736
clientConn, serverConn := net.Pipe()
737-
conn, err := tailnet.NewConn(&tailnet.Options{
737+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
738738
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
739739
DERPMap: api.DERPMap,
740740
Logger: api.Logger.Named("tailnet"),

‎coderd/wsconncache/wsconncache_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func setupAgent(t *testing.T, manifest agentsdk.Manifest, ptyTimeout time.Durati
178178
t.Cleanup(func() {
179179
_ = closer.Close()
180180
})
181-
conn, err := tailnet.NewConn(&tailnet.Options{
181+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
182182
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
183183
DERPMap: manifest.DERPMap,
184184
Logger: slogtest.Make(t, nil).Named("tailnet").Leveled(slog.LevelDebug),

‎codersdk/workspaceagents.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ func (c *Client) DialWorkspaceAgent(ctx context.Context, agentID uuid.UUID, opti
246246
if ok {
247247
header = headerTransport.Header()
248248
}
249-
conn, err := tailnet.NewConn(&tailnet.Options{
249+
conn, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
250250
Addresses: []netip.Prefix{netip.PrefixFrom(ip, 128)},
251251
DERPMap: connInfo.DERPMap,
252252
DERPHeader: &header,

‎tailnet/conn.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,17 @@ func init() {
7474
envknob.Setenv("TS_DEBUG_TRIM_WIREGUARD", "false")
7575
}
7676

77+
// ConnType identifies the type of tailnet connection. This determines whether the connection actively establishes
78+
// the wireguard handshake (client) or passively waits for a handshake (agent). The reason this matters is that if they
79+
// both initiate the handshake, the handshakes can cross in the network, and they have to try again with a backoff,
80+
// which can add significant time to establish the connection
81+
type ConnType int
82+
83+
const (
84+
ConnTypeClient ConnType = iota
85+
ConnTypeAgent
86+
)
87+
7788
type Options struct {
7889
Addresses []netip.Prefix
7990
DERPMap *tailcfg.DERPMap
@@ -87,7 +98,7 @@ type Options struct {
8798
}
8899

89100
// NewConn constructs a new Wireguard server that will accept connections from the addresses provided.
90-
func NewConn(options *Options) (conn *Conn, err error) {
101+
func NewConn(connType ConnType, options *Options) (conn *Conn, err error) {
91102
if options == nil {
92103
options = &Options{}
93104
}
@@ -238,6 +249,7 @@ func NewConn(options *Options) (conn *Conn, err error) {
238249

239250
dialContext, dialCancel := context.WithCancel(context.Background())
240251
server := &Conn{
252+
connType: connType,
241253
blockEndpoints: options.BlockEndpoints,
242254
dialContext: dialContext,
243255
dialCancel: dialCancel,
@@ -346,6 +358,7 @@ func IPFromUUID(uid uuid.UUID) netip.Addr {
346358

347359
// Conn is an actively listening Wireguard connection.
348360
type Conn struct {
361+
connType ConnType
349362
dialContext context.Context
350363
dialCancel context.CancelFunc
351364
mutex sync.Mutex
@@ -483,7 +496,7 @@ func (c *Conn) UpdateNodes(nodes []*Node, replacePeers bool) error {
483496
Endpoints: node.Endpoints,
484497
DERP: fmt.Sprintf("%s:%d", tailcfg.DerpMagicIP, node.PreferredDERP),
485498
Hostinfo: hostinfo.New().View(),
486-
KeepAlive: true,
499+
KeepAlive: c.connType == ConnTypeClient,
487500
}
488501
if c.blockEndpoints {
489502
peerNode.Endpoints = nil

‎tailnet/conn_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func TestTailnet(t *testing.T) {
2626
derpMap, _ := tailnettest.RunDERPAndSTUN(t)
2727
t.Run("InstantClose", func(t *testing.T) {
2828
t.Parallel()
29-
conn, err := tailnet.NewConn(&tailnet.Options{
29+
conn, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
3030
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
3131
Logger: logger.Named("w1"),
3232
DERPMap: derpMap,
@@ -39,15 +39,16 @@ func TestTailnet(t *testing.T) {
3939
t.Parallel()
4040
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
4141
defer cancel()
42+
// we make w1 the agent and w2 the client because we call AwaitReachable() from w2
4243
w1IP := tailnet.IP()
43-
w1, err := tailnet.NewConn(&tailnet.Options{
44+
w1, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
4445
Addresses: []netip.Prefix{netip.PrefixFrom(w1IP, 128)},
4546
Logger: logger.Named("w1"),
4647
DERPMap: derpMap,
4748
})
4849
require.NoError(t, err)
4950

50-
w2, err := tailnet.NewConn(&tailnet.Options{
51+
w2, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
5152
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
5253
Logger: logger.Named("w2"),
5354
DERPMap: derpMap,
@@ -107,15 +108,15 @@ func TestTailnet(t *testing.T) {
107108

108109
w1IP := tailnet.IP()
109110
derpMap := tailnettest.RunDERPOnlyWebSockets(t)
110-
w1, err := tailnet.NewConn(&tailnet.Options{
111+
w1, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
111112
Addresses: []netip.Prefix{netip.PrefixFrom(w1IP, 128)},
112113
Logger: logger.Named("w1"),
113114
DERPMap: derpMap,
114115
BlockEndpoints: true,
115116
})
116117
require.NoError(t, err)
117118

118-
w2, err := tailnet.NewConn(&tailnet.Options{
119+
w2, err := tailnet.NewConn(tailnet.ConnTypeClient, &tailnet.Options{
119120
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
120121
Logger: logger.Named("w2"),
121122
DERPMap: derpMap,
@@ -177,7 +178,7 @@ func TestConn_PreferredDERP(t *testing.T) {
177178
defer cancel()
178179
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
179180
derpMap, _ := tailnettest.RunDERPAndSTUN(t)
180-
conn, err := tailnet.NewConn(&tailnet.Options{
181+
conn, err := tailnet.NewConn(tailnet.ConnTypeAgent, &tailnet.Options{
181182
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
182183
Logger: logger.Named("w1"),
183184
DERPMap: derpMap,

0 commit comments

Comments
 (0)