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") + } +}