From 824027305a2b986b523b5b29dab7b96dba4475aa Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Tue, 25 Nov 2025 15:05:04 -0800 Subject: [PATCH] cmd/tailscale/cli,ipn,all: make peer relay server port a *uint16 In preparation for exposing its configuration via ipn.ConfigVAlpha, change {Masked}Prefs.RelayServerPort from *int to *uint16. This takes a defensive stance against invalid inputs at JSON decode time. 'tailscale set --relay-server-port' is currently the only input to this pref, and has always sanitized input to fit within a uint16. Updates tailscale/corp#34591 Signed-off-by: Jordan Whited --- cmd/tailscale/cli/set.go | 2 +- feature/relayserver/relayserver.go | 6 +-- feature/relayserver/relayserver_test.go | 58 ++++++++++++------------- ipn/ipn_clone.go | 2 +- ipn/ipn_view.go | 8 ++-- ipn/prefs.go | 10 ++--- ipn/prefs_test.go | 2 +- net/udprelay/server.go | 6 +-- net/udprelay/status/status.go | 5 ++- 9 files changed, 48 insertions(+), 51 deletions(-) diff --git a/cmd/tailscale/cli/set.go b/cmd/tailscale/cli/set.go index c2316580c..31662392f 100644 --- a/cmd/tailscale/cli/set.go +++ b/cmd/tailscale/cli/set.go @@ -249,7 +249,7 @@ func runSet(ctx context.Context, args []string) (retErr error) { if err != nil { return fmt.Errorf("failed to set relay server port: %v", err) } - maskedPrefs.Prefs.RelayServerPort = ptr.To(int(uport)) + maskedPrefs.Prefs.RelayServerPort = ptr.To(uint16(uport)) } if setArgs.relayServerStaticEndpoints != "" { diff --git a/feature/relayserver/relayserver.go b/feature/relayserver/relayserver.go index e85576e50..4f23ae18e 100644 --- a/feature/relayserver/relayserver.go +++ b/feature/relayserver/relayserver.go @@ -69,7 +69,7 @@ func servePeerRelayDebugSessions(h *localapi.Handler, w http.ResponseWriter, r * // imported. func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) { e := &extension{ - newServerFn: func(logf logger.Logf, port int, onlyStaticAddrPorts bool) (relayServer, error) { + newServerFn: func(logf logger.Logf, port uint16, onlyStaticAddrPorts bool) (relayServer, error) { return udprelay.NewServer(logf, port, onlyStaticAddrPorts) }, logf: logger.WithPrefix(logf, featureName+": "), @@ -93,7 +93,7 @@ type relayServer interface { // extension is an [ipnext.Extension] managing the relay server on platforms // that import this package. type extension struct { - newServerFn func(logf logger.Logf, port int, onlyStaticAddrPorts bool) (relayServer, error) // swappable for tests + newServerFn func(logf logger.Logf, port uint16, onlyStaticAddrPorts bool) (relayServer, error) // swappable for tests logf logger.Logf ec *eventbus.Client respPub *eventbus.Publisher[magicsock.UDPRelayAllocResp] @@ -101,7 +101,7 @@ type extension struct { mu syncs.Mutex // guards the following fields shutdown bool // true if Shutdown() has been called rs relayServer // nil when disabled - port *int // ipn.Prefs.RelayServerPort, nil if disabled + port *uint16 // ipn.Prefs.RelayServerPort, nil if disabled staticEndpoints views.Slice[netip.AddrPort] // ipn.Prefs.RelayServerStaticEndpoints derpMapView tailcfg.DERPMapView // latest seen over the eventbus hasNodeAttrDisableRelayServer bool // [tailcfg.NodeAttrDisableRelayServer] diff --git a/feature/relayserver/relayserver_test.go b/feature/relayserver/relayserver_test.go index d77d2df26..807306c70 100644 --- a/feature/relayserver/relayserver_test.go +++ b/feature/relayserver/relayserver_test.go @@ -23,15 +23,15 @@ import ( ) func Test_extension_profileStateChanged(t *testing.T) { - prefsWithPortOne := ipn.Prefs{RelayServerPort: ptr.To(1)} + prefsWithPortOne := ipn.Prefs{RelayServerPort: ptr.To(uint16(1))} prefsWithNilPort := ipn.Prefs{RelayServerPort: nil} prefsWithPortOneRelayEndpoints := ipn.Prefs{ - RelayServerPort: ptr.To(1), + RelayServerPort: ptr.To(uint16(1)), RelayServerStaticEndpoints: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:7777")}, } type fields struct { - port *int + port *uint16 staticEndpoints views.Slice[netip.AddrPort] rs relayServer } @@ -43,7 +43,7 @@ func Test_extension_profileStateChanged(t *testing.T) { name string fields fields args args - wantPort *int + wantPort *uint16 wantRelayServerFieldNonNil bool wantRelayServerFieldMutated bool wantEndpoints []netip.AddrPort @@ -51,28 +51,28 @@ func Test_extension_profileStateChanged(t *testing.T) { { name: "no changes non-nil port previously running", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), }, args: args{ prefs: prefsWithPortOne.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: false, }, { name: "set addr ports unchanged port previously running", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), }, args: args{ prefs: prefsWithPortOneRelayEndpoints.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: false, wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints, @@ -87,7 +87,7 @@ func Test_extension_profileStateChanged(t *testing.T) { prefs: prefsWithPortOneRelayEndpoints.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints, @@ -95,7 +95,7 @@ func Test_extension_profileStateChanged(t *testing.T) { { name: "clear addr ports unchanged port previously running", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), staticEndpoints: views.SliceOf(prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints), rs: mockRelayServerNotZeroVal(), }, @@ -103,7 +103,7 @@ func Test_extension_profileStateChanged(t *testing.T) { prefs: prefsWithPortOne.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: false, wantEndpoints: nil, @@ -111,7 +111,7 @@ func Test_extension_profileStateChanged(t *testing.T) { { name: "prefs port nil", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), }, args: args{ prefs: prefsWithNilPort.View(), @@ -124,7 +124,7 @@ func Test_extension_profileStateChanged(t *testing.T) { { name: "prefs port nil previously running", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), }, args: args{ @@ -138,54 +138,54 @@ func Test_extension_profileStateChanged(t *testing.T) { { name: "prefs port changed", fields: fields{ - port: ptr.To(2), + port: ptr.To(uint16(2)), }, args: args{ prefs: prefsWithPortOne.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, }, { name: "prefs port changed previously running", fields: fields{ - port: ptr.To(2), + port: ptr.To(uint16(2)), rs: mockRelayServerNotZeroVal(), }, args: args{ prefs: prefsWithPortOne.View(), sameNode: true, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, }, { name: "sameNode false", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), }, args: args{ prefs: prefsWithPortOne.View(), sameNode: false, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, }, { name: "sameNode false previously running", fields: fields{ - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), }, args: args{ prefs: prefsWithPortOne.View(), sameNode: false, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, }, @@ -198,7 +198,7 @@ func Test_extension_profileStateChanged(t *testing.T) { prefs: prefsWithPortOne.View(), sameNode: false, }, - wantPort: ptr.To(1), + wantPort: ptr.To(uint16(1)), wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, }, @@ -211,7 +211,7 @@ func Test_extension_profileStateChanged(t *testing.T) { t.Fatal(err) } e := ipne.(*extension) - e.newServerFn = func(logf logger.Logf, port int, onlyStaticAddrPorts bool) (relayServer, error) { + e.newServerFn = func(logf logger.Logf, port uint16, onlyStaticAddrPorts bool) (relayServer, error) { return &mockRelayServer{}, nil } e.port = tt.fields.port @@ -271,7 +271,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { tests := []struct { name string shutdown bool - port *int + port *uint16 rs relayServer hasNodeAttrDisableRelayServer bool wantRelayServerFieldNonNil bool @@ -280,7 +280,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { { name: "want running", shutdown: false, - port: ptr.To(1), + port: ptr.To(uint16(1)), hasNodeAttrDisableRelayServer: false, wantRelayServerFieldNonNil: true, wantRelayServerFieldMutated: true, @@ -288,7 +288,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { { name: "want running previously running", shutdown: false, - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), hasNodeAttrDisableRelayServer: false, wantRelayServerFieldNonNil: true, @@ -297,7 +297,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { { name: "shutdown true", shutdown: true, - port: ptr.To(1), + port: ptr.To(uint16(1)), hasNodeAttrDisableRelayServer: false, wantRelayServerFieldNonNil: false, wantRelayServerFieldMutated: false, @@ -305,7 +305,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { { name: "shutdown true previously running", shutdown: true, - port: ptr.To(1), + port: ptr.To(uint16(1)), rs: mockRelayServerNotZeroVal(), hasNodeAttrDisableRelayServer: false, wantRelayServerFieldNonNil: false, @@ -354,7 +354,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { t.Fatal(err) } e := ipne.(*extension) - e.newServerFn = func(logf logger.Logf, port int, onlyStaticAddrPorts bool) (relayServer, error) { + e.newServerFn = func(logf logger.Logf, port uint16, onlyStaticAddrPorts bool) (relayServer, error) { return &mockRelayServer{}, nil } e.shutdown = tt.shutdown diff --git a/ipn/ipn_clone.go b/ipn/ipn_clone.go index fae85adee..4bf78b40b 100644 --- a/ipn/ipn_clone.go +++ b/ipn/ipn_clone.go @@ -102,7 +102,7 @@ var _PrefsCloneNeedsRegeneration = Prefs(struct { PostureChecking bool NetfilterKind string DriveShares []*drive.Share - RelayServerPort *int + RelayServerPort *uint16 RelayServerStaticEndpoints []netip.AddrPort AllowSingleHosts marshalAsTrueInJSON Persist *persist.Persist diff --git a/ipn/ipn_view.go b/ipn/ipn_view.go index aac8cb4d7..4157ec76e 100644 --- a/ipn/ipn_view.go +++ b/ipn/ipn_view.go @@ -441,10 +441,8 @@ func (v PrefsView) DriveShares() views.SliceView[*drive.Share, drive.ShareView] // RelayServerPort is the UDP port number for the relay server to bind to, // on all interfaces. A non-nil zero value signifies a random unused port // should be used. A nil value signifies relay server functionality -// should be disabled. This field is currently experimental, and therefore -// no guarantees are made about its current naming and functionality when -// non-nil/enabled. -func (v PrefsView) RelayServerPort() views.ValuePointer[int] { +// should be disabled. +func (v PrefsView) RelayServerPort() views.ValuePointer[uint16] { return views.ValuePointerOf(v.ж.RelayServerPort) } @@ -506,7 +504,7 @@ var _PrefsViewNeedsRegeneration = Prefs(struct { PostureChecking bool NetfilterKind string DriveShares []*drive.Share - RelayServerPort *int + RelayServerPort *uint16 RelayServerStaticEndpoints []netip.AddrPort AllowSingleHosts marshalAsTrueInJSON Persist *persist.Persist diff --git a/ipn/prefs.go b/ipn/prefs.go index 6f3cb65f8..9f98465d2 100644 --- a/ipn/prefs.go +++ b/ipn/prefs.go @@ -283,10 +283,8 @@ type Prefs struct { // RelayServerPort is the UDP port number for the relay server to bind to, // on all interfaces. A non-nil zero value signifies a random unused port // should be used. A nil value signifies relay server functionality - // should be disabled. This field is currently experimental, and therefore - // no guarantees are made about its current naming and functionality when - // non-nil/enabled. - RelayServerPort *int `json:",omitempty"` + // should be disabled. + RelayServerPort *uint16 `json:",omitempty"` // RelayServerStaticEndpoints are static IP:port endpoints to advertise as // candidates for relay connections. Only relevant when RelayServerPort is @@ -694,7 +692,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool { p.PostureChecking == p2.PostureChecking && slices.EqualFunc(p.DriveShares, p2.DriveShares, drive.SharesEqual) && p.NetfilterKind == p2.NetfilterKind && - compareIntPtrs(p.RelayServerPort, p2.RelayServerPort) && + compareUint16Ptrs(p.RelayServerPort, p2.RelayServerPort) && slices.Equal(p.RelayServerStaticEndpoints, p2.RelayServerStaticEndpoints) } @@ -715,7 +713,7 @@ func (ap AppConnectorPrefs) Pretty() string { return "" } -func compareIntPtrs(a, b *int) bool { +func compareUint16Ptrs(a, b *uint16) bool { if (a == nil) != (b == nil) { return false } diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index cf0750706..aa152843a 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -78,7 +78,7 @@ func TestPrefsEqual(t *testing.T) { have, prefsHandles) } - relayServerPort := func(port int) *int { + relayServerPort := func(port uint16) *uint16 { return &port } nets := func(strs ...string) (ns []netip.Prefix) { diff --git a/net/udprelay/server.go b/net/udprelay/server.go index b260955e0..e7ca24960 100644 --- a/net/udprelay/server.go +++ b/net/udprelay/server.go @@ -309,7 +309,7 @@ func (e *serverEndpoint) isBound() bool { // onlyStaticAddrPorts is true, then dynamic addr:port discovery will be // disabled, and only addr:port's set via [Server.SetStaticAddrPorts] will be // used. -func NewServer(logf logger.Logf, port int, onlyStaticAddrPorts bool) (s *Server, err error) { +func NewServer(logf logger.Logf, port uint16, onlyStaticAddrPorts bool) (s *Server, err error) { s = &Server{ logf: logf, disco: key.NewDisco(), @@ -526,9 +526,9 @@ func trySetUDPSocketOptions(pconn nettype.PacketConn, logf logger.Logf) { // [magicsock.RebindingConn], which would also remove the need for // [singlePacketConn], as [magicsock.RebindingConn] also handles fallback to // single packet syscall operations. -func (s *Server) listenOn(port int) error { +func (s *Server) listenOn(port uint16) error { for _, network := range []string{"udp4", "udp6"} { - uc, err := net.ListenUDP(network, &net.UDPAddr{Port: port}) + uc, err := net.ListenUDP(network, &net.UDPAddr{Port: int(port)}) if err != nil { if network == "udp4" { return err diff --git a/net/udprelay/status/status.go b/net/udprelay/status/status.go index 3866efada..9ed9a0d2a 100644 --- a/net/udprelay/status/status.go +++ b/net/udprelay/status/status.go @@ -14,8 +14,9 @@ import ( type ServerStatus struct { // UDPPort is the UDP port number that the peer relay server forwards over, // as configured by the user with 'tailscale set --relay-server-port='. - // If the port has not been configured, UDPPort will be nil. - UDPPort *int + // If the port has not been configured, UDPPort will be nil. A non-nil zero + // value signifies the user has opted for a random unused port. + UDPPort *uint16 // Sessions is a slice of detailed status information about each peer // relay session that this node's peer relay server is involved with. It // may be empty.