From 246c82a658b35851f5ca07fe503ce6b20b39e806 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 19 Jul 2026 03:29:47 +0000 Subject: [PATCH] derp, wgengine: let clients advertise an opaque app name to DERP servers Add an AppName field to the DERP ClientInfo so DERP servers can attribute connections to the application making them, primarily for best effort stats purposes. The value is plumbed per engine instance rather than via a process global, so a process hosting multiple stacks can attribute each one's DERP connections separately: wgengine.Config.DERPAppName flows through magicsock.Options and derphttp.Client into the naclbox-sealed ClientInfo JSON. Old servers ignore the unknown field. There are no callers in the tree yet setting the name. Updates tailscale/corp#24454 Signed-off-by: Brad Fitzpatrick Change-Id: Ia7d3e9c2b6f8140e5a9d7c3b2e6f1a8d4c0b5e9f --- derp/derp_client.go | 17 +++++- derp/derphttp/derphttp_client.go | 3 ++ derp/derpserver/derpserver.go | 9 ++++ derp/derpserver/fortest.go | 30 +++++++++++ wgengine/magicsock/derp.go | 1 + wgengine/magicsock/magicsock.go | 6 +++ wgengine/userspace.go | 6 +++ wgengine/userspace_test.go | 90 ++++++++++++++++++++++++++++++++ 8 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 derp/derpserver/fortest.go diff --git a/derp/derp_client.go b/derp/derp_client.go index cc792a2f2..e907b7657 100644 --- a/derp/derp_client.go +++ b/derp/derp_client.go @@ -33,6 +33,7 @@ type Client struct { meshKey key.DERPMesh canAckPings bool isProber bool + appName string wmu sync.Mutex // hold while writing to bw bw *bufio.Writer @@ -60,6 +61,7 @@ type clientOpt struct { ServerPub key.NodePublic CanAckPings bool IsProber bool + AppName string } // MeshKey returns a ClientOpt to pass to the DERP server during connect to get @@ -84,6 +86,13 @@ func CanAckPings(v bool) ClientOpt { return clientOptFunc(func(o *clientOpt) { o.CanAckPings = v }) } +// AppName returns a ClientOpt to set an opaque app name string to +// advertise to the DERP server for stats purposes. It is sent to the +// server in the ClientInfo. +func AppName(name string) ClientOpt { + return clientOptFunc(func(o *clientOpt) { o.AppName = name }) +} + func NewClient(privateKey key.NodePrivate, nc Conn, brw *bufio.ReadWriter, logf logger.Logf, opts ...ClientOpt) (*Client, error) { var opt clientOpt for _, o := range opts { @@ -106,6 +115,7 @@ func newClient(privateKey key.NodePrivate, nc Conn, brw *bufio.ReadWriter, logf meshKey: opt.MeshKey, canAckPings: opt.CanAckPings, isProber: opt.IsProber, + appName: opt.AppName, clock: tstime.StdClock{}, } if opt.ServerPub.IsZero() { @@ -179,6 +189,10 @@ type ClientInfo struct { // IsProber is whether this client is a prober. IsProber bool `json:",omitempty"` + + // AppName is an optional opaque app name string the client + // advertises to the server for stats purposes. + AppName string `json:",omitempty"` } // Equal reports if two clientInfo values are equal. @@ -186,7 +200,7 @@ func (c *ClientInfo) Equal(other *ClientInfo) bool { if c == nil || other == nil { return c == other } - if c.Version != other.Version || c.CanAckPings != other.CanAckPings || c.IsProber != other.IsProber { + if c.Version != other.Version || c.CanAckPings != other.CanAckPings || c.IsProber != other.IsProber || c.AppName != other.AppName { return false } return c.MeshKey.Equal(other.MeshKey) @@ -198,6 +212,7 @@ func (c *Client) sendClientKey() error { MeshKey: c.meshKey, CanAckPings: c.canAckPings, IsProber: c.isProber, + AppName: c.appName, }) if err != nil { return err diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index 2b048b512..7ba21983e 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -60,6 +60,7 @@ type Client struct { DNSCache *dnscache.Resolver // optional; nil means no caching MeshKey key.DERPMesh // optional; for trusted clients IsProber bool // optional; for probers to optional declare themselves as such + AppName string // optional; opaque app name to advertise to the server for stats // WatchConnectionChanges is whether the client wishes to subscribe to // notifications about clients connecting & disconnecting. @@ -413,6 +414,7 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien derp.MeshKey(c.MeshKey), derp.CanAckPings(c.canAckPings), derp.IsProber(c.IsProber), + derp.AppName(c.AppName), ) if err != nil { return nil, 0, err @@ -558,6 +560,7 @@ func (c *Client) connect(ctx context.Context, caller string) (client *derp.Clien derp.ServerPublicKey(serverPub), derp.CanAckPings(c.canAckPings), derp.IsProber(c.IsProber), + derp.AppName(c.AppName), ) if err != nil { return nil, 0, err diff --git a/derp/derpserver/derpserver.go b/derp/derpserver/derpserver.go index d46399693..d0e2f6dc0 100644 --- a/derp/derpserver/derpserver.go +++ b/derp/derpserver/derpserver.go @@ -141,6 +141,12 @@ type Server struct { debug bool localClient local.Client + // onClientInfoForTest, if non-nil, is called with each connecting + // client's key and ClientInfo. It is set (before the server accepts + // any connections) via forTest.SetOnClientInfo and is nil outside + // of tests. + onClientInfoForTest func(key.NodePublic, derp.ClientInfo) + // Counters: packetsSent, bytesSent expvar.Int packetsRecv, bytesRecv expvar.Int @@ -1061,6 +1067,9 @@ func (s *Server) accept(ctx context.Context, nc derp.Conn, brw *bufio.ReadWriter if s.debug { c.debug = true } + if f := s.onClientInfoForTest; f != nil { + f(clientKey, c.info) + } s.registerClient(c) defer s.unregisterClient(c) diff --git a/derp/derpserver/fortest.go b/derp/derpserver/fortest.go new file mode 100644 index 000000000..c8741b0bc --- /dev/null +++ b/derp/derpserver/fortest.go @@ -0,0 +1,30 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package derpserver + +import ( + "tailscale.com/derp" + "tailscale.com/types/key" + "tailscale.com/util/testenv" +) + +// forTest is an unexported type to hide the test-only methods on +// [Server] from godoc. +type forTest struct{ s *Server } + +// ForTest returns a handle to test-only methods on s. The resulting +// type is unexported to make it very obvious in godoc that this is +// not stable API. This method panics if called outside of tests, +// which also centralizes all must-be-in-tests validation. +func (s *Server) ForTest() forTest { + testenv.AssertInTest() + return forTest{s} +} + +// SetOnClientInfo sets a func to be called with each connecting +// client's key and the ClientInfo it sent. It must be called before +// the server accepts any connections. +func (f forTest) SetOnClientInfo(fn func(key.NodePublic, derp.ClientInfo)) { + f.s.onClientInfoForTest = fn +} diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 72c75db5a..214c61f0d 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -414,6 +414,7 @@ func (c *Conn) derpWriteChanForRegion(regionID int, peer key.NodePublic) chan de return derpMap.Regions[regionID] }) dc.HealthTracker = c.health + dc.AppName = c.derpAppName if c.extraRootCAs != nil { dc.TLSConfig = &tls.Config{RootCAs: c.extraRootCAs} } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index e9fd2b38b..adf6fac8b 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -170,6 +170,7 @@ type Conn struct { health *health.Tracker // or nil extraRootCAs *x509.CertPool // additional trusted root CAs; or nil controlKnobs *controlknobs.Knobs // or nil + derpAppName string // or empty, see Options.DERPAppName // ================================================================ // No locking required to access these fields, either because @@ -489,6 +490,10 @@ type Options struct { // for TLS connections to DERP servers. ExtraRootCAs *x509.CertPool + // DERPAppName, if non-empty, is an opaque app name string to + // advertise to DERP servers for stats purposes. + DERPAppName string + // Metrics specifies the metrics registry to record metrics to. Metrics *usermetric.Registry @@ -705,6 +710,7 @@ func NewConn(opts Options) (*Conn, error) { c.netMon = opts.NetMon c.health = opts.HealthTracker c.extraRootCAs = opts.ExtraRootCAs + c.derpAppName = opts.DERPAppName c.getPeerByKey = opts.PeerByKeyFunc if err := c.rebind(keepCurrentPort); err != nil { diff --git a/wgengine/userspace.go b/wgengine/userspace.go index c049e0e9b..c9bae72ee 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -210,6 +210,11 @@ type Config struct { // connections (e.g. DERP). Passed through to magicsock. ExtraRootCAs *x509.CertPool + // DERPAppName, if non-empty, is an opaque app name string to + // advertise to DERP servers for stats purposes. It is passed + // through to magicsock. + DERPAppName string + // ControlKnobs is the set of control plane-provied knobs // to use. // If nil, defaults are used. @@ -425,6 +430,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) NetMon: e.netMon, HealthTracker: e.health, ExtraRootCAs: conf.ExtraRootCAs, + DERPAppName: conf.DERPAppName, Metrics: conf.Metrics, ControlKnobs: conf.ControlKnobs, PeerByKeyFunc: e.PeerByKey, diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index a5569d329..efe2e04bb 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -4,9 +4,13 @@ package wgengine import ( + "crypto/tls" "errors" "fmt" "math/rand" + "net" + "net/http" + "net/http/httptest" "net/netip" "os" "runtime" @@ -19,12 +23,15 @@ import ( "go4.org/mem" "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/control/controlknobs" + "tailscale.com/derp" + "tailscale.com/derp/derpserver" "tailscale.com/envknob" "tailscale.com/health" "tailscale.com/net/dns" "tailscale.com/net/dns/resolver" "tailscale.com/net/netaddr" "tailscale.com/net/netmon" + "tailscale.com/net/stun/stuntest" "tailscale.com/tailcfg" "tailscale.com/types/dnstype" "tailscale.com/types/key" @@ -553,3 +560,86 @@ func TestCloseWaitsForLinkChange(t *testing.T) { t.Fatal("Close returned with link change work still in flight") } } + +// TestDERPAppNamePlumbing tests that Config.DERPAppName makes it all +// the way from the engine config to the ClientInfo received by an +// in-process DERP server. +func TestDERPAppNamePlumbing(t *testing.T) { + const appName = "app-name-plumbing-test" + + priv := key.NewNode() + infoCh := make(chan derp.ClientInfo, 1) + + derpSrv := derpserver.New(key.NewNode(), t.Logf) + derpSrv.ForTest().SetOnClientInfo(func(k key.NodePublic, info derp.ClientInfo) { + if k != priv.Public() { + return + } + select { + case infoCh <- info: + default: + } + }) + httpsrv := httptest.NewUnstartedServer(derpserver.Handler(derpSrv)) + httpsrv.Config.ErrorLog = logger.StdLogger(t.Logf) + httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler)) + httpsrv.StartTLS() + t.Cleanup(func() { + httpsrv.CloseClientConnections() + httpsrv.Close() + derpSrv.Close() + }) + + stunAddr, stunCleanup := stuntest.Serve(t) + t.Cleanup(stunCleanup) + + 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: "none", + STUNPort: stunAddr.Port, + DERPPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port, + InsecureForTests: true, + }}, + }, + }, + } + + bus := eventbustest.NewBus(t) + noopDNS, err := dns.NewNoopManager() + if err != nil { + t.Fatal(err) + } + e, err := NewUserspaceEngine(t.Logf, Config{ + HealthTracker: health.NewTracker(bus), + Metrics: new(usermetric.Registry), + EventBus: bus, + DNS: noopDNS, + DERPAppName: appName, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(e.Close) + + if err := e.Reconfig(&wgcfg.Config{PrivateKey: priv}, &router.Config{}, &dns.Config{}); err != nil { + t.Fatalf("Reconfig: %v", err) + } + e.(*userspaceEngine).magicConn.SetDERPMap(derpMap) + + select { + case info := <-infoCh: + if info.AppName != appName { + t.Fatalf("ClientInfo.AppName = %q; want %q", info.AppName, appName) + } + case <-time.After(10 * time.Second): + t.Fatal("timeout waiting for engine to connect to the test DERP server") + } +}