Skip to content

fix: direct embedded derp traffic directly to the server #6595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 14, 2023
+34 −38
Merged

fix: direct embedded derp traffic directly to the server #6595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
t.Cleanup(stunCleanup)

derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp")))
derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug)))
derpServer.SetMeshKey("test-key")

// match default with cli default
Expand Down
51 changes: 15 additions & 36 deletions coderd/workspaceagents.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package coderd

import (
"bufio"
"context"
"database/sql"
"encoding/json"
Expand Down Expand Up @@ -428,51 +429,29 @@ func (api *API) dialWorkspaceAgentTailnet(r *http.Request, agentID uuid.UUID) (*
ctx := r.Context()
clientConn, serverConn := net.Pipe()

derpMap := api.DERPMap.Clone()
for _, region := range derpMap.Regions {
if !region.EmbeddedRelay {
continue
}
var node *tailcfg.DERPNode
for _, n := range region.Nodes {
if n.STUNOnly {
continue
}
node = n
break
}
if node == nil {
continue
}
// TODO: This should dial directly to execute the
// DERP server instead of contacting localhost.
//
// This requires modification of Tailscale internals
// to pipe through a proxy function per-region, so
// this is an easy and mostly reliable hack for now.
cloned := node.Clone()
// Add p for proxy.
// This first node supports TLS.
cloned.Name += "p"
cloned.IPv4 = "127.0.0.1"
cloned.InsecureForTests = true
region.Nodes = append(region.Nodes, cloned.Clone())
// This second node forces HTTP.
cloned.Name += "-http"
cloned.ForceHTTP = true
region.Nodes = append(region.Nodes, cloned)
}

conn, err := tailnet.NewConn(&tailnet.Options{
Addresses: []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
DERPMap: derpMap,
DERPMap: api.DERPMap,
Logger: api.Logger.Named("tailnet"),
})
if err != nil {
_ = clientConn.Close()
_ = serverConn.Close()
return nil, xerrors.Errorf("create tailnet conn: %w", err)
}
conn.SetDERPRegionDialer(func(_ context.Context, region *tailcfg.DERPRegion) net.Conn {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a comment above in the code that seems outdated now?

		// TODO: This should dial directly to execute the
		// DERP server instead of contacting localhost.
		//
		// This requires modification of Tailscale internals
		// to pipe through a proxy function per-region, so
		// this is an easy and mostly reliable hack for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point! I'm happy we're able to take out a TODO!

if !region.EmbeddedRelay {
return nil
}
left, right := net.Pipe()
go func() {
defer left.Close()
defer right.Close()
brw := bufio.NewReadWriter(bufio.NewReader(right), bufio.NewWriter(right))
api.DERPServer.Accept(ctx, right, brw, r.RemoteAddr)
}()
return left
})

sendNodes, _ := tailnet.ServeCoordinator(clientConn, func(node []*tailnet.Node) error {
err = conn.UpdateNodes(node, true)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.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.20230307022319-1e5e724a3949
replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20230314023417-d9efcc0ac972

// Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
// See: https://github.com/coder/coder/issues/3371
Expand Down
12 changes: 12 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,18 @@ github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHui
github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
github.com/coder/tailscale v1.1.1-0.20230307022319-1e5e724a3949 h1:8WfMfRTDaEpnmhCJWfFQ7JHz19GyP+EgFgLGu5ngdek=
github.com/coder/tailscale v1.1.1-0.20230307022319-1e5e724a3949/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230313184322-307d4b9ef4e1 h1:kOh+1rBcbahdodSiNBioO4g47m3Ef8CagNEX8U7/RyY=
github.com/coder/tailscale v1.1.1-0.20230313184322-307d4b9ef4e1/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230313192237-8d691a009b20 h1:FFPv3xLsAT+n8chkePxB/VwqFwn4NL8GV8Lk97efUP0=
github.com/coder/tailscale v1.1.1-0.20230313192237-8d691a009b20/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230313194848-e281f646c460 h1:WYnHzxQ1DsgDYyyS6i8tgdpjiwS1LrXJjwXcOVkU/3g=
github.com/coder/tailscale v1.1.1-0.20230313194848-e281f646c460/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230313195101-c6534848756b h1:3xWbBCpQ+CqVP2Myh1YhCw8cJbMi6mjkDl4/x6YkiUw=
github.com/coder/tailscale v1.1.1-0.20230313195101-c6534848756b/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230314021518-0cda9db154be h1:b0ZTDPpV/fAdkuS/V59cawnq+5vcXzkvXPMG/eZcRx0=
github.com/coder/tailscale v1.1.1-0.20230314021518-0cda9db154be/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/tailscale v1.1.1-0.20230314023417-d9efcc0ac972 h1:193YGsJz8hc4yxqAclE36paKl+9CQ6KGLgdleIguCVE=
github.com/coder/tailscale v1.1.1-0.20230314023417-d9efcc0ac972/go.mod h1:jpg+77g19FpXL43U1VoIqoSg1K/Vh5CVxycGldQ8KhA=
github.com/coder/terraform-provider-coder v0.6.15 h1:Llvh4RwxSQ/goy7ToTOeHf3tdEz+79qbyOh61hNnJs0=
github.com/coder/terraform-provider-coder v0.6.15/go.mod h1:UIfU3bYNeSzJJvHyJ30tEKjD6Z9utloI+HUM/7n94CY=
github.com/containerd/aufs v0.0.0-20200908144142-dab0cbea06f4/go.mod h1:nukgQABAEopAHvB6j7cnP5zJ+/3aVcE7hCYqvIwAHyE=
Expand Down
5 changes: 5 additions & 0 deletions tailnet/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,11 @@ func (c *Conn) SetDERPMap(derpMap *tailcfg.DERPMap) {
c.wireguardEngine.SetNetworkMap(&netMapCopy)
}

// SetDERPRegionDialer updates the dialer to use for connecting to DERP regions.
func (c *Conn) SetDERPRegionDialer(dialer func(ctx context.Context, region *tailcfg.DERPRegion) net.Conn) {
c.magicConn.SetDERPRegionDialer(dialer)
}

func (c *Conn) RemoveAllPeers() error {
c.mutex.Lock()
defer c.mutex.Unlock()
Expand Down