From 73ac7b961d0d35fec683bd26c4245d617cef1ee2 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 14 Feb 2024 14:09:08 +0000 Subject: [PATCH 1/3] fix: never send local endpoints if disabled --- go.mod | 2 +- go.sum | 4 ++-- tailnet/conn.go | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 42a4dfd0b6621..5665da6ea940a 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,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.20231205095743-61c97bad8c8b +replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20240214140224-3788ab894ba1 // Fixes a race-condition in coder/wgtunnel. // Upstream PR: https://github.com/WireGuard/wireguard-go/pull/85 diff --git a/go.sum b/go.sum index b663dac08d6ac..901d6511d80fe 100644 --- a/go.sum +++ b/go.sum @@ -202,8 +202,8 @@ github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= github.com/coder/retry v1.5.1/go.mod h1:blHMk9vs6LkoRT9ZHyuZo360cufXEhrxqvEzeMtRGoY= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuOD6a/zVP3rcxezNsoDseTUw= github.com/coder/ssh v0.0.0-20231128192721-70855dedb788/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= -github.com/coder/tailscale v1.1.1-0.20231205095743-61c97bad8c8b h1:ut/aL6oI8TjGdg4JI8+bKB9w5j73intbe0dJAmcmYyQ= -github.com/coder/tailscale v1.1.1-0.20231205095743-61c97bad8c8b/go.mod h1:L8tPrwSi31RAMEMV8rjb0vYTGs7rXt8rAHbqY/p41j4= +github.com/coder/tailscale v1.1.1-0.20240214140224-3788ab894ba1 h1:A7dZHNidAVH6Kxn5D3hTEH+iRO8slnM0aRer6/cxlyE= +github.com/coder/tailscale v1.1.1-0.20240214140224-3788ab894ba1/go.mod h1:L8tPrwSi31RAMEMV8rjb0vYTGs7rXt8rAHbqY/p41j4= github.com/coder/terraform-provider-coder v0.14.2 h1:CfQyQH9bOTI3cauxKJyPCUDkIcHOaMyFifxehkkkz/8= github.com/coder/terraform-provider-coder v0.14.2/go.mod h1:pACHRoXSHBGyY696mLeQ1hR/Ag1G2wFk5bw0mT5Zp2g= github.com/coder/wgtunnel v0.1.13-0.20231127054351-578bfff9b92a h1:KhR9LUVllMZ+e9lhubZ1HNrtJDgH5YLoTvpKwmrGag4= diff --git a/tailnet/conn.go b/tailnet/conn.go index 129567260f307..fccd6b70c8833 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -171,6 +171,7 @@ func NewConn(options *Options) (conn *Conn, err error) { magicConn := sys.MagicSock.Get() magicConn.SetDERPForceWebsockets(options.DERPForceWebSockets) + magicConn.SetBlockEndpoints(options.BlockEndpoints) if options.DERPHeader != nil { magicConn.SetDERPHeader(options.DERPHeader.Clone()) } @@ -346,6 +347,7 @@ func (c *Conn) SetDERPForceWebSockets(v bool) { func (c *Conn) SetBlockEndpoints(blockEndpoints bool) { c.configMaps.setBlockEndpoints(blockEndpoints) c.nodeUpdater.setBlockEndpoints(blockEndpoints) + c.magicConn.SetBlockEndpoints(blockEndpoints) } // SetDERPRegionDialer updates the dialer to use for connecting to DERP regions. From 2c2ca6bd31b1ccf0affbf1e484e5df3d7302e18b Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 14 Feb 2024 14:40:08 +0000 Subject: [PATCH 2/3] Add 10s delay test to ensure endpoints don't appear --- tailnet/conn_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/tailnet/conn_test.go b/tailnet/conn_test.go index b904e98fe6173..852423078ef0f 100644 --- a/tailnet/conn_test.go +++ b/tailnet/conn_test.go @@ -4,6 +4,7 @@ import ( "context" "net/netip" "testing" + "time" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -412,9 +413,78 @@ parentLoop: require.True(t, client2.AwaitReachable(awaitReachableCtx4, ip)) } +func TestConn_BlockEndpoints(t *testing.T) { + t.Parallel() + logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) + + derpMap, _ := tailnettest.RunDERPAndSTUN(t) + + // Setup conn 1. + ip1 := tailnet.IP() + conn1, err := tailnet.NewConn(&tailnet.Options{ + Addresses: []netip.Prefix{netip.PrefixFrom(ip1, 128)}, + Logger: logger.Named("w1"), + DERPMap: derpMap, + BlockEndpoints: true, + }) + require.NoError(t, err) + defer func() { + err := conn1.Close() + assert.NoError(t, err) + }() + + // Setup conn 2. + ip2 := tailnet.IP() + conn2, err := tailnet.NewConn(&tailnet.Options{ + Addresses: []netip.Prefix{netip.PrefixFrom(ip2, 128)}, + Logger: logger.Named("w2"), + DERPMap: derpMap, + BlockEndpoints: true, + }) + require.NoError(t, err) + defer func() { + err := conn2.Close() + assert.NoError(t, err) + }() + + // Connect them together and wait for them to be reachable. + nodes1 := stitch(t, conn2, conn1) + nodes2 := stitch(t, conn1, conn2) + awaitReachableCtx, awaitReachableCancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer awaitReachableCancel() + require.True(t, conn1.AwaitReachable(awaitReachableCtx, ip2)) + + // Watch for 10 seconds to ensure that neither peer has any endpoints. + // There's no good way to force endpoint updates, so we just wait. +parentLoop: + for { + select { + case node := <-nodes1: + require.Empty(t, node.Endpoints, "conn1 got endpoints") + case node := <-nodes2: + require.Empty(t, node.Endpoints, "conn2 got endpoints") + case <-time.After(testutil.WaitShort): + break parentLoop + } + } + + // Double check that both peers don't have endpoints for the other peer + // according to magicsock. + conn1Status, ok := conn1.Status().Peer[conn2.Node().Key] + require.True(t, ok) + require.Empty(t, conn1Status.Addrs) + require.Empty(t, conn1Status.CurAddr) + conn2Status, ok := conn2.Status().Peer[conn1.Node().Key] + require.True(t, ok) + require.Empty(t, conn2Status.Addrs) + require.Empty(t, conn2Status.CurAddr) +} + // stitch sends node updates from src Conn as peer updates to dst Conn. Sort of // like the Coordinator would, but without actually needing a Coordinator. -func stitch(t *testing.T, dst, src *tailnet.Conn) { +func stitch(t *testing.T, dst, src *tailnet.Conn) <-chan *tailnet.Node { + // Buffered to avoid blocking the callback. + out := make(chan *tailnet.Node, 50) srcID := uuid.New() src.SetNodeCallback(func(node *tailnet.Node) { pn, err := tailnet.NodeToProto(node) @@ -427,5 +497,11 @@ func stitch(t *testing.T, dst, src *tailnet.Conn) { Kind: proto.CoordinateResponse_PeerUpdate_NODE, }}) assert.NoError(t, err) + + select { + case out <- node: + default: + } }) + return out } From 97f5accd3c3059dec36068641b6a56026c679381 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Tue, 20 Feb 2024 03:56:04 +0000 Subject: [PATCH 3/3] Comments --- tailnet/conn_test.go | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/tailnet/conn_test.go b/tailnet/conn_test.go index 852423078ef0f..823b7303dbc49 100644 --- a/tailnet/conn_test.go +++ b/tailnet/conn_test.go @@ -448,25 +448,15 @@ func TestConn_BlockEndpoints(t *testing.T) { }() // Connect them together and wait for them to be reachable. - nodes1 := stitch(t, conn2, conn1) - nodes2 := stitch(t, conn1, conn2) + stitch(t, conn2, conn1) + stitch(t, conn1, conn2) awaitReachableCtx, awaitReachableCancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer awaitReachableCancel() require.True(t, conn1.AwaitReachable(awaitReachableCtx, ip2)) - // Watch for 10 seconds to ensure that neither peer has any endpoints. - // There's no good way to force endpoint updates, so we just wait. -parentLoop: - for { - select { - case node := <-nodes1: - require.Empty(t, node.Endpoints, "conn1 got endpoints") - case node := <-nodes2: - require.Empty(t, node.Endpoints, "conn2 got endpoints") - case <-time.After(testutil.WaitShort): - break parentLoop - } - } + // Wait 10s for endpoints to potentially be sent over Disco. There's no way + // to force Disco to send endpoints immediately. + time.Sleep(10 * time.Second) // Double check that both peers don't have endpoints for the other peer // according to magicsock. @@ -482,9 +472,7 @@ parentLoop: // stitch sends node updates from src Conn as peer updates to dst Conn. Sort of // like the Coordinator would, but without actually needing a Coordinator. -func stitch(t *testing.T, dst, src *tailnet.Conn) <-chan *tailnet.Node { - // Buffered to avoid blocking the callback. - out := make(chan *tailnet.Node, 50) +func stitch(t *testing.T, dst, src *tailnet.Conn) { srcID := uuid.New() src.SetNodeCallback(func(node *tailnet.Node) { pn, err := tailnet.NodeToProto(node) @@ -497,11 +485,5 @@ func stitch(t *testing.T, dst, src *tailnet.Conn) <-chan *tailnet.Node { Kind: proto.CoordinateResponse_PeerUpdate_NODE, }}) assert.NoError(t, err) - - select { - case out <- node: - default: - } }) - return out }