net/udprelay{/endpoint}, all: move ServerEndpoint to independent pkg (#15934)

ServerEndpoint will be used within magicsock and potentially elsewhere,
which should be possible without needing to import the server
implementation itself.

Updates tailscale/corp#27502

Signed-off-by: Jordan Whited <jordan@tailscale.com>

cmd/tailscaled/depaware.txt

@@ -350,6 +350,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/clientupdate/distsign+
         tailscale.com/net/tstun                                      from tailscale.com/cmd/tailscaled+
         tailscale.com/net/udprelay                                   from tailscale.com/feature/relayserver
+        tailscale.com/net/udprelay/endpoint                          from tailscale.com/feature/relayserver+
         tailscale.com/omit                                           from tailscale.com/ipn/conffile
         tailscale.com/paths                                          from tailscale.com/client/local+
      💣 tailscale.com/portlist                                       from tailscale.com/ipn/ipnlocal

disco/disco.go

@@ -406,8 +406,8 @@ func parseBindUDPRelayEndpointAnswer(ver uint8, p []byte) (m *BindUDPRelayEndpoi
 // involving [BindUDPRelayEndpoint], [BindUDPRelayEndpointChallenge], and
 // [BindUDPRelayEndpointAnswer].
 //
-// CallMeMaybeVia mirrors [tailscale.com/net/udprelay.ServerEndpoint], which
+// CallMeMaybeVia mirrors [tailscale.com/net/udprelay/endpoint.ServerEndpoint],
-// contains field documentation.
+// which contains field documentation.
 //
 // The recipient may choose to not open a path back if it's already happy with
 // its path. Direct connections, e.g. [CallMeMaybe]-signaled, take priority over
@@ -416,17 +416,17 @@ func parseBindUDPRelayEndpointAnswer(ver uint8, p []byte) (m *BindUDPRelayEndpoi
 // This message type is currently considered experimental and is not yet tied to
 // a [tailscale.com/tailcfg.CapabilityVersion].
 type CallMeMaybeVia struct {
-	// ServerDisco is [tailscale.com/net/udprelay.ServerEndpoint.ServerDisco]
+	// ServerDisco is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.ServerDisco]
 	ServerDisco key.DiscoPublic
-	// LamportID is [tailscale.com/net/udprelay.ServerEndpoint.LamportID]
+	// LamportID is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.LamportID]
 	LamportID uint64
-	// VNI is [tailscale.com/net/udprelay.ServerEndpoint.VNI]
+	// VNI is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.VNI]
 	VNI uint32
-	// BindLifetime is [tailscale.com/net/udprelay.ServerEndpoint.BindLifetime]
+	// BindLifetime is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.BindLifetime]
 	BindLifetime time.Duration
-	// SteadyStateLifetime is [tailscale.com/net/udprelay.ServerEndpoint.SteadyStateLifetime]
+	// SteadyStateLifetime is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.SteadyStateLifetime]
 	SteadyStateLifetime time.Duration
-	// AddrPorts is [tailscale.com/net/udprelay.ServerEndpoint.AddrPorts]
+	// AddrPorts is [tailscale.com/net/udprelay/endpoint.ServerEndpoint.AddrPorts]
 	AddrPorts []netip.AddrPort
 }
 

feature/relayserver/relayserver.go

@@ -19,6 +19,7 @@ import (
 	"tailscale.com/ipn/ipnext"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/net/udprelay"
+	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -57,7 +58,7 @@ type extension struct {
 
 // relayServer is the interface of [udprelay.Server].
 type relayServer interface {
-	AllocateEndpoint(discoA key.DiscoPublic, discoB key.DiscoPublic) (udprelay.ServerEndpoint, error)
+	AllocateEndpoint(discoA key.DiscoPublic, discoB key.DiscoPublic) (endpoint.ServerEndpoint, error)
 	Close() error
 }
 

feature/relayserver/relayserver_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 
 	"tailscale.com/ipn"
-	"tailscale.com/net/udprelay"
+	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/types/key"
 	"tailscale.com/types/ptr"
 )
@@ -17,8 +17,8 @@ type fakeRelayServer struct{}
 
 func (f *fakeRelayServer) Close() error { return nil }
 
-func (f *fakeRelayServer) AllocateEndpoint(_, _ key.DiscoPublic) (udprelay.ServerEndpoint, error) {
+func (f *fakeRelayServer) AllocateEndpoint(_, _ key.DiscoPublic) (endpoint.ServerEndpoint, error) {
-	return udprelay.ServerEndpoint{}, errors.New("fake relay server")
+	return endpoint.ServerEndpoint{}, errors.New("fake relay server")
 }
 
 func Test_extension_profileStateChanged(t *testing.T) {

net/udprelay/endpoint/endpoint.go

@@ -0,0 +1,55 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package endpoint contains types relating to UDP relay server endpoints. It
+// does not import tailscale.com/net/udprelay.
+package endpoint
+
+import (
+	"net/netip"
+
+	"tailscale.com/tstime"
+	"tailscale.com/types/key"
+)
+
+// ServerEndpoint contains details for an endpoint served by a
+// [tailscale.com/net/udprelay.Server].
+type ServerEndpoint struct {
+	// ServerDisco is the Server's Disco public key used as part of the 3-way
+	// bind handshake. Server will use the same ServerDisco for its lifetime.
+	// ServerDisco value in combination with LamportID value represents a
+	// unique ServerEndpoint allocation.
+	ServerDisco key.DiscoPublic
+
+	// LamportID is unique and monotonically non-decreasing across
+	// ServerEndpoint allocations for the lifetime of Server. It enables clients
+	// to dedup and resolve allocation event order. Clients may race to allocate
+	// on the same Server, and signal ServerEndpoint details via alternative
+	// channels, e.g. DERP. Additionally, Server.AllocateEndpoint() requests may
+	// not result in a new allocation depending on existing server-side endpoint
+	// state. Therefore, where clients have local, existing state that contains
+	// ServerDisco and LamportID values matching a newly learned endpoint, these
+	// can be considered one and the same. If ServerDisco is equal, but
+	// LamportID is unequal, LamportID comparison determines which
+	// ServerEndpoint was allocated most recently.
+	LamportID uint64
+
+	// AddrPorts are the IP:Port candidate pairs the Server may be reachable
+	// over.
+	AddrPorts []netip.AddrPort
+
+	// VNI (Virtual Network Identifier) is the Geneve header VNI the Server
+	// will use for transmitted packets, and expects for received packets
+	// associated with this endpoint.
+	VNI uint32
+
+	// BindLifetime is amount of time post-allocation the Server will consider
+	// the endpoint active while it has yet to be bound via 3-way bind handshake
+	// from both client parties.
+	BindLifetime tstime.GoDuration
+
+	// SteadyStateLifetime is the amount of time post 3-way bind handshake from
+	// both client parties the Server will consider the endpoint active lacking
+	// bidirectional data flow.
+	SteadyStateLifetime tstime.GoDuration
+}

net/udprelay/server.go

@@ -21,20 +21,21 @@ import (
 	"go4.org/mem"
 	"tailscale.com/disco"
 	"tailscale.com/net/packet"
+	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/tstime"
 	"tailscale.com/types/key"
 )
 
 const (
 	// defaultBindLifetime is somewhat arbitrary. We attempt to account for
-	// high latency between client and Server, and high latency between
+	// high latency between client and [Server], and high latency between
-	// clients over side channels, e.g. DERP, used to exchange ServerEndpoint
+	// clients over side channels, e.g. DERP, used to exchange
-	// details. So, a total of 3 paths with potentially high latency. Using a
+	// [endpoint.ServerEndpoint] details. So, a total of 3 paths with
-	// conservative 10s "high latency" bounds for each path we end up at a 30s
+	// potentially high latency. Using a conservative 10s "high latency" bounds
-	// total. It is worse to set an aggressive bind lifetime as this may lead
+	// for each path we end up at a 30s total. It is worse to set an aggressive
-	// to path discovery failure, vs dealing with a slight increase of Server
+	// bind lifetime as this may lead to path discovery failure, vs dealing with
-	// resource utilization (VNIs, RAM, etc) while tracking endpoints that won't
+	// a slight increase of [Server] resource utilization (VNIs, RAM, etc) while
-	// bind.
+	// tracking endpoints that won't bind.
 	defaultBindLifetime        = time.Second * 30
 	defaultSteadyStateLifetime = time.Minute * 5
 )
@@ -82,49 +83,8 @@ func newPairOfDiscoPubKeys(discoA, discoB key.DiscoPublic) pairOfDiscoPubKeys {
 	return pair
 }
 
-// ServerEndpoint contains the Server's endpoint details.
+// serverEndpoint contains Server-internal [endpoint.ServerEndpoint] state.
-type ServerEndpoint struct {
+// serverEndpoint methods are not thread-safe.
-	// ServerDisco is the Server's Disco public key used as part of the 3-way
-	// bind handshake. Server will use the same ServerDisco for its lifetime.
-	// ServerDisco value in combination with LamportID value represents a
-	// unique ServerEndpoint allocation.
-	ServerDisco key.DiscoPublic
-
-	// LamportID is unique and monotonically non-decreasing across
-	// ServerEndpoint allocations for the lifetime of Server. It enables clients
-	// to dedup and resolve allocation event order. Clients may race to allocate
-	// on the same Server, and signal ServerEndpoint details via alternative
-	// channels, e.g. DERP. Additionally, Server.AllocateEndpoint() requests may
-	// not result in a new allocation depending on existing server-side endpoint
-	// state. Therefore, where clients have local, existing state that contains
-	// ServerDisco and LamportID values matching a newly learned endpoint, these
-	// can be considered one and the same. If ServerDisco is equal, but
-	// LamportID is unequal, LamportID comparison determines which
-	// ServerEndpoint was allocated most recently.
-	LamportID uint64
-
-	// AddrPorts are the IP:Port candidate pairs the Server may be reachable
-	// over.
-	AddrPorts []netip.AddrPort
-
-	// VNI (Virtual Network Identifier) is the Geneve header VNI the Server
-	// will use for transmitted packets, and expects for received packets
-	// associated with this endpoint.
-	VNI uint32
-
-	// BindLifetime is amount of time post-allocation the Server will consider
-	// the endpoint active while it has yet to be bound via 3-way bind handshake
-	// from both client parties.
-	BindLifetime tstime.GoDuration
-
-	// SteadyStateLifetime is the amount of time post 3-way bind handshake from
-	// both client parties the Server will consider the endpoint active lacking
-	// bidirectional data flow.
-	SteadyStateLifetime tstime.GoDuration
-}
-
-// serverEndpoint contains Server-internal ServerEndpoint state. serverEndpoint
-// methods are not thread-safe.
 type serverEndpoint struct {
 	// discoPubKeys contains the key.DiscoPublic of the served clients. The
 	// indexing of this array aligns with the following fields, e.g.
@@ -308,10 +268,11 @@ func (e *serverEndpoint) isBound() bool {
 		e.handshakeState[1] == disco.BindUDPRelayHandshakeStateAnswerReceived
 }
 
-// NewServer constructs a Server listening on 0.0.0.0:'port'. IPv6 is not yet
+// NewServer constructs a [Server] listening on 0.0.0.0:'port'. IPv6 is not yet
 // supported. Port may be 0, and what ultimately gets bound is returned as
 // 'boundPort'. Supplied 'addrs' are joined with 'boundPort' and returned as
-// ServerEndpoint.AddrPorts in response to Server.AllocateEndpoint() requests.
+// [endpoint.ServerEndpoint.AddrPorts] in response to Server.AllocateEndpoint()
+// requests.
 //
 // TODO: IPv6 support
 // TODO: dynamic addrs:port discovery
@@ -454,30 +415,30 @@ func (s *Server) packetReadLoop() {
 
 var ErrServerClosed = errors.New("server closed")
 
-// AllocateEndpoint allocates a [ServerEndpoint] for the provided pair of
+// AllocateEndpoint allocates an [endpoint.ServerEndpoint] for the provided pair
-// [key.DiscoPublic]'s. If an allocation already exists for discoA and discoB it
+// of [key.DiscoPublic]'s. If an allocation already exists for discoA and discoB
-// is returned without modification/reallocation. AllocateEndpoint returns
+// it is returned without modification/reallocation. AllocateEndpoint returns
 // [ErrServerClosed] if the server has been closed.
-func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (ServerEndpoint, error) {
+func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (endpoint.ServerEndpoint, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	if s.closed {
-		return ServerEndpoint{}, ErrServerClosed
+		return endpoint.ServerEndpoint{}, ErrServerClosed
 	}
 
 	if discoA.Compare(s.discoPublic) == 0 || discoB.Compare(s.discoPublic) == 0 {
-		return ServerEndpoint{}, fmt.Errorf("client disco equals server disco: %s", s.discoPublic.ShortString())
+		return endpoint.ServerEndpoint{}, fmt.Errorf("client disco equals server disco: %s", s.discoPublic.ShortString())
 	}
 
 	pair := newPairOfDiscoPubKeys(discoA, discoB)
 	e, ok := s.byDisco[pair]
 	if ok {
 		// Return the existing allocation. Clients can resolve duplicate
-		// [ServerEndpoint]'s via [ServerEndpoint.LamportID].
+		// [endpoint.ServerEndpoint]'s via [endpoint.ServerEndpoint.LamportID].
 		//
 		// TODO: consider ServerEndpoint.BindLifetime -= time.Now()-e.allocatedAt
 		// to give the client a more accurate picture of the bind window.
-		return ServerEndpoint{
+		return endpoint.ServerEndpoint{
 			ServerDisco:         s.discoPublic,
 			AddrPorts:           s.addrPorts,
 			VNI:                 e.vni,
@@ -488,7 +449,7 @@ func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (ServerEndpoin
 	}
 
 	if len(s.vniPool) == 0 {
-		return ServerEndpoint{}, errors.New("VNI pool exhausted")
+		return endpoint.ServerEndpoint{}, errors.New("VNI pool exhausted")
 	}
 
 	s.lamportID++
@@ -506,7 +467,7 @@ func (s *Server) AllocateEndpoint(discoA, discoB key.DiscoPublic) (ServerEndpoin
 	s.byDisco[pair] = e
 	s.byVNI[e.vni] = e
 
-	return ServerEndpoint{
+	return endpoint.ServerEndpoint{
 		ServerDisco:         s.discoPublic,
 		AddrPorts:           s.addrPorts,
 		VNI:                 e.vni,

net/udprelay/server_test.go

@@ -17,6 +17,7 @@ import (
 	"go4.org/mem"
 	"tailscale.com/disco"
 	"tailscale.com/net/packet"
+	"tailscale.com/net/udprelay/endpoint"
 	"tailscale.com/tstime"
 	"tailscale.com/types/key"
 )
@@ -259,7 +260,7 @@ func TestServerEndpointJSONUnmarshal(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			var out ServerEndpoint
+			var out endpoint.ServerEndpoint
 			err := json.Unmarshal(tt.json, &out)
 			if tt.wantErr != (err != nil) {
 				t.Fatalf("wantErr: %v (err == nil): %v", tt.wantErr, err == nil)
@@ -274,11 +275,11 @@ func TestServerEndpointJSONUnmarshal(t *testing.T) {
 func TestServerEndpointJSONMarshal(t *testing.T) {
 	tests := []struct {
 		name           string
-		serverEndpoint ServerEndpoint
+		serverEndpoint endpoint.ServerEndpoint
 	}{
 		{
 			name: "valid roundtrip",
-			serverEndpoint: ServerEndpoint{
+			serverEndpoint: endpoint.ServerEndpoint{
 				ServerDisco:         key.NewDisco().Public(),
 				LamportID:           uint64(math.MaxUint64),
 				AddrPorts:           []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:1"), netip.MustParseAddrPort("127.0.0.2:2")},
@@ -295,7 +296,7 @@ func TestServerEndpointJSONMarshal(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			var got ServerEndpoint
+			var got endpoint.ServerEndpoint
 			err = json.Unmarshal(b, &got)
 			if err != nil {
 				t.Fatal(err)