Skip to content

fix: use DERPPort in DERP URL if using node.HostName #35

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
Aug 24, 2023
Merged

fix: use DERPPort in DERP URL if using node.HostName #35

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
31 changes: 17 additions & 14 deletions derp/derphttp/derphttp_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,23 +260,26 @@ func (c *Client) urlString(node *tailcfg.DERPNode) string {
if c.url != nil {
return c.url.String()
}
if node.HostName == "" {
url := &url.URL{
Scheme: "https",
Host: fmt.Sprintf("%s:%d", node.IPv4, node.DERPPort),
Path: "/derp",
}
if node.ForceHTTP {
url.Scheme = "http"
}
return url.String()
}

proto := "https"
url := &url.URL{
Scheme: "https",
Host: node.IPv4,
Path: "/derp",
}
if debugUseDERPHTTP() || node.ForceHTTP {
proto = "http"
url.Scheme = "http"
}
return fmt.Sprintf("%s://%s/derp", proto, node.HostName)
if url.Host == "" {
url.Host = node.HostName
}

// Add the DERPPort to the URL host, unless the host already contains a port
// or the DERPPort is the default port for the protocol.
if node.DERPPort != 0 && !strings.Contains(url.Host, ":") && ((url.Scheme == "https" && node.DERPPort != 443) || (url.Scheme == "http" && node.DERPPort != 80)) {
url.Host = fmt.Sprintf("%s:%d", url.Host, node.DERPPort)
}

return url.String()
}

// AddressFamilySelector decides whether IPv6 is preferred for
Expand Down
124 changes: 124 additions & 0 deletions wgengine/magicsock/magicsock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package magicsock

import (
"bufio"
"bytes"
"context"
crand "crypto/rand"
Expand All @@ -12,6 +13,7 @@ import (
"errors"
"fmt"
"io"
"log"
"math/rand"
"net"
"net/http"
Expand All @@ -37,6 +39,7 @@ import (
"golang.org/x/net/icmp"
"golang.org/x/net/ipv4"
"golang.org/x/net/ipv6"
"nhooyr.io/websocket"
"tailscale.com/cmd/testwrapper/flakytest"
"tailscale.com/derp"
"tailscale.com/derp/derphttp"
Expand All @@ -48,6 +51,7 @@ import (
"tailscale.com/net/ping"
"tailscale.com/net/stun/stuntest"
"tailscale.com/net/tstun"
"tailscale.com/net/wsconn"
"tailscale.com/tailcfg"
"tailscale.com/tstest"
"tailscale.com/tstest/natlab"
Expand Down Expand Up @@ -161,7 +165,16 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
return newMagicStackWithKey(t, logf, l, derpMap, privateKey)
}

func newMagicStackFunc(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, updateConnFunc func(ms *Conn)) *magicStack {
privateKey := key.NewNode()
return newMagicStackWithKeyFunc(t, logf, l, derpMap, privateKey, updateConnFunc)
}

func newMagicStackWithKey(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, privateKey key.NodePrivate) *magicStack {
return newMagicStackWithKeyFunc(t, logf, l, derpMap, privateKey, nil)
}

func newMagicStackWithKeyFunc(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, privateKey key.NodePrivate, updateConnFunc func(ms *Conn)) *magicStack {
t.Helper()

epCh := make(chan []tailcfg.Endpoint, 100) // arbitrary
Expand All @@ -186,6 +199,11 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, l nettype.PacketListen

wgLogger := wglog.NewLogger(logf)
dev := wgcfg.NewDevice(tsTun, conn.Bind(), wgLogger.DeviceLogger)

if updateConnFunc != nil {
updateConnFunc(conn)
}

dev.Up()

// Wait for magicsock to connect up to DERP.
Expand Down Expand Up @@ -2876,3 +2894,109 @@ func TestAddrForSendLockedForWireGuardOnly(t *testing.T) {
}
}
}

// Copied from cmd/derper
func addWebSocketSupport(s *derp.Server, base http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
up := strings.ToLower(r.Header.Get("Upgrade"))

// Very early versions of Tailscale set "Upgrade: WebSocket" but didn't actually
// speak WebSockets (they still assumed DERP's binary framing). So to distinguish
// clients that actually want WebSockets, look for an explicit "derp" subprotocol.
if up != "websocket" || !strings.Contains(r.Header.Get("Sec-Websocket-Protocol"), "derp") {
base.ServeHTTP(w, r)
return
}

c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
Subprotocols: []string{"derp"},
OriginPatterns: []string{"*"},
// Disable compression because we transmit WireGuard messages that
// are not compressible.
// Additionally, Safari has a broken implementation of compression
// (see https://github.com/nhooyr/websocket/issues/218) that makes
// enabling it actively harmful.
CompressionMode: websocket.CompressionDisabled,
})
if err != nil {
log.Printf("websocket.Accept: %v", err)
return
}
defer c.Close(websocket.StatusInternalError, "closing")
if c.Subprotocol() != "derp" {
c.Close(websocket.StatusPolicyViolation, "client must speak the derp subprotocol")
return
}
wc := wsconn.NetConn(r.Context(), c, websocket.MessageBinary)
brw := bufio.NewReadWriter(bufio.NewReader(wc), bufio.NewWriter(wc))
s.Accept(r.Context(), wc, brw, r.RemoteAddr)
})
}

func TestDERPForceWebsockets(t *testing.T) {
logf, closeLogf := logger.LogfCloser(t.Logf)
defer closeLogf()

// Create a DERP server manually, without a STUN server and with a custom
// handler.
derpServer := derp.NewServer(key.NewNode(), logf)
derpHandler := derphttp.Handler(derpServer)
derpHandler = addWebSocketSupport(derpServer, derpHandler)

var upgradeCount int64
httpsrv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
up := r.Header.Get("Upgrade")
if up != "" {
if up != "websocket" {
t.Errorf("unexpected upgrade header: %q", up)
} else {
atomic.AddInt64(&upgradeCount, 1)
}
}

derpHandler.ServeHTTP(w, r)
}))
httpsrv.Config.ErrorLog = logger.StdLogger(logf)
httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
httpsrv.StartTLS()
t.Cleanup(func() {
httpsrv.CloseClientConnections()
httpsrv.Close()
derpServer.Close()
})

derpMap := &tailcfg.DERPMap{
Regions: map[int]*tailcfg.DERPRegion{
1: {
RegionID: 1,
RegionCode: "test",
Nodes: []*tailcfg.DERPNode{
{
Name: "t1",
RegionID: 1,
HostName: "test-node.unused",
IPv4: "127.0.0.1",
IPv6: "",
STUNPort: -1,
DERPPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port,
InsecureForTests: true,
},
},
},
},
}

m := &natlab.Machine{Name: "m1"}
ms := newMagicStackFunc(t, logger.WithPrefix(logf, "conn1: "), m, derpMap, func(ms *Conn) {
ms.SetDERPForceWebsockets(true)
})
defer ms.Close()

if len(ms.conn.activeDerp) == 0 {
t.Errorf("unexpected DERP empty got: %v want: >0", len(ms.conn.activeDerp))
}

if atomic.LoadInt64(&upgradeCount) == 0 {
t.Errorf("no websocket upgrade requests seen")
}
}