archive
/
tailscale
mirror of https://github.com/tailscale/tailscale/
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

cmd/tailscale/cli,ipn,all: make peer relay server port a *uint16

Browse Source 
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 <jordan@tailscale.com>
pull/17986/head
Jordan Whited 1 week ago committed by Jordan Whited
parent 53476ce872
commit 824027305a
9 changed files with 48 additions and 51 deletions
Show Stats Download Patch File Download Diff File

2
cmd/tailscale/cli/set.go
Unescape Escape View File

@ -249,7 +249,7 @@ func runSet(ctx context.Context, args []string) (retErr error) {
if err != nil { if err != nil {
return fmt.Errorf("failed to set relay server port: %v", err) 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 != "" { if setArgs.relayServerStaticEndpoints != "" {

6
feature/relayserver/relayserver.go
Unescape Escape View File

@ -69,7 +69,7 @@ func servePeerRelayDebugSessions(h *localapi.Handler, w http.ResponseWriter, r *
// imported. // imported.
func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) { func newExtension(logf logger.Logf, sb ipnext.SafeBackend) (ipnext.Extension, error) {
e := &extension{ 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) return udprelay.NewServer(logf, port, onlyStaticAddrPorts)
}, },
logf: logger.WithPrefix(logf, featureName+": "), logf: logger.WithPrefix(logf, featureName+": "),
@ -93,7 +93,7 @@ type relayServer interface {
// extension is an [ipnext.Extension] managing the relay server on platforms // extension is an [ipnext.Extension] managing the relay server on platforms
// that import this package. // that import this package.
type extension struct { 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 logf logger.Logf
ec *eventbus.Client ec *eventbus.Client
respPub *eventbus.Publisher[magicsock.UDPRelayAllocResp] respPub *eventbus.Publisher[magicsock.UDPRelayAllocResp]
@ -101,7 +101,7 @@ type extension struct {
mu syncs.Mutex // guards the following fields mu syncs.Mutex // guards the following fields
shutdown bool // true if Shutdown() has been called shutdown bool // true if Shutdown() has been called
rs relayServer // nil when disabled 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 staticEndpoints views.Slice[netip.AddrPort] // ipn.Prefs.RelayServerStaticEndpoints
derpMapView tailcfg.DERPMapView // latest seen over the eventbus derpMapView tailcfg.DERPMapView // latest seen over the eventbus
hasNodeAttrDisableRelayServer bool // [tailcfg.NodeAttrDisableRelayServer] hasNodeAttrDisableRelayServer bool // [tailcfg.NodeAttrDisableRelayServer]

58
feature/relayserver/relayserver_test.go
Unescape Escape View File

@ -23,15 +23,15 @@ import (
) )
func Test_extension_profileStateChanged(t *testing.T) { 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} prefsWithNilPort := ipn.Prefs{RelayServerPort: nil}
prefsWithPortOneRelayEndpoints := ipn.Prefs{ prefsWithPortOneRelayEndpoints := ipn.Prefs{
RelayServerPort: ptr.To(1), RelayServerPort: ptr.To(uint16(1)),
RelayServerStaticEndpoints: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:7777")}, RelayServerStaticEndpoints: []netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:7777")},
} }
type fields struct { type fields struct {
port *int port *uint16
staticEndpoints views.Slice[netip.AddrPort] staticEndpoints views.Slice[netip.AddrPort]
rs relayServer rs relayServer
} }
@ -43,7 +43,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
name string name string
fields fields fields fields
args args args args
wantPort *int wantPort *uint16
wantRelayServerFieldNonNil bool wantRelayServerFieldNonNil bool
wantRelayServerFieldMutated bool wantRelayServerFieldMutated bool
wantEndpoints []netip.AddrPort wantEndpoints []netip.AddrPort
@ -51,28 +51,28 @@ func Test_extension_profileStateChanged(t *testing.T) {
{ {
name: "no changes non-nil port previously running", name: "no changes non-nil port previously running",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
args: args{ args: args{
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: false, wantRelayServerFieldMutated: false,
}, },
{ {
name: "set addr ports unchanged port previously running", name: "set addr ports unchanged port previously running",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
args: args{ args: args{
prefs: prefsWithPortOneRelayEndpoints.View(), prefs: prefsWithPortOneRelayEndpoints.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: false, wantRelayServerFieldMutated: false,
wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints, wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints,
@ -87,7 +87,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
prefs: prefsWithPortOneRelayEndpoints.View(), prefs: prefsWithPortOneRelayEndpoints.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints, wantEndpoints: prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints,
@ -95,7 +95,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
{ {
name: "clear addr ports unchanged port previously running", name: "clear addr ports unchanged port previously running",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
staticEndpoints: views.SliceOf(prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints), staticEndpoints: views.SliceOf(prefsWithPortOneRelayEndpoints.RelayServerStaticEndpoints),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
@ -103,7 +103,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: false, wantRelayServerFieldMutated: false,
wantEndpoints: nil, wantEndpoints: nil,
@ -111,7 +111,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
{ {
name: "prefs port nil", name: "prefs port nil",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
}, },
args: args{ args: args{
prefs: prefsWithNilPort.View(), prefs: prefsWithNilPort.View(),
@ -124,7 +124,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
{ {
name: "prefs port nil previously running", name: "prefs port nil previously running",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
args: args{ args: args{
@ -138,54 +138,54 @@ func Test_extension_profileStateChanged(t *testing.T) {
{ {
name: "prefs port changed", name: "prefs port changed",
fields: fields{ fields: fields{
port: ptr.To(2), port: ptr.To(uint16(2)),
}, },
args: args{ args: args{
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
}, },
{ {
name: "prefs port changed previously running", name: "prefs port changed previously running",
fields: fields{ fields: fields{
port: ptr.To(2), port: ptr.To(uint16(2)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
args: args{ args: args{
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: true, sameNode: true,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
}, },
{ {
name: "sameNode false", name: "sameNode false",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
}, },
args: args{ args: args{
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: false, sameNode: false,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
}, },
{ {
name: "sameNode false previously running", name: "sameNode false previously running",
fields: fields{ fields: fields{
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
}, },
args: args{ args: args{
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: false, sameNode: false,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
}, },
@ -198,7 +198,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
prefs: prefsWithPortOne.View(), prefs: prefsWithPortOne.View(),
sameNode: false, sameNode: false,
}, },
wantPort: ptr.To(1), wantPort: ptr.To(uint16(1)),
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
}, },
@ -211,7 +211,7 @@ func Test_extension_profileStateChanged(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
e := ipne.(*extension) 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 return &mockRelayServer{}, nil
} }
e.port = tt.fields.port e.port = tt.fields.port
@ -271,7 +271,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
shutdown bool shutdown bool
port *int port *uint16
rs relayServer rs relayServer
hasNodeAttrDisableRelayServer bool hasNodeAttrDisableRelayServer bool
wantRelayServerFieldNonNil bool wantRelayServerFieldNonNil bool
@ -280,7 +280,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
{ {
name: "want running", name: "want running",
shutdown: false, shutdown: false,
port: ptr.To(1), port: ptr.To(uint16(1)),
hasNodeAttrDisableRelayServer: false, hasNodeAttrDisableRelayServer: false,
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
wantRelayServerFieldMutated: true, wantRelayServerFieldMutated: true,
@ -288,7 +288,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
{ {
name: "want running previously running", name: "want running previously running",
shutdown: false, shutdown: false,
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
hasNodeAttrDisableRelayServer: false, hasNodeAttrDisableRelayServer: false,
wantRelayServerFieldNonNil: true, wantRelayServerFieldNonNil: true,
@ -297,7 +297,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
{ {
name: "shutdown true", name: "shutdown true",
shutdown: true, shutdown: true,
port: ptr.To(1), port: ptr.To(uint16(1)),
hasNodeAttrDisableRelayServer: false, hasNodeAttrDisableRelayServer: false,
wantRelayServerFieldNonNil: false, wantRelayServerFieldNonNil: false,
wantRelayServerFieldMutated: false, wantRelayServerFieldMutated: false,
@ -305,7 +305,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
{ {
name: "shutdown true previously running", name: "shutdown true previously running",
shutdown: true, shutdown: true,
port: ptr.To(1), port: ptr.To(uint16(1)),
rs: mockRelayServerNotZeroVal(), rs: mockRelayServerNotZeroVal(),
hasNodeAttrDisableRelayServer: false, hasNodeAttrDisableRelayServer: false,
wantRelayServerFieldNonNil: false, wantRelayServerFieldNonNil: false,
@ -354,7 +354,7 @@ func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
e := ipne.(*extension) 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 return &mockRelayServer{}, nil
} }
e.shutdown = tt.shutdown e.shutdown = tt.shutdown

2
ipn/ipn_clone.go
Unescape Escape View File

@ -102,7 +102,7 @@ var _PrefsCloneNeedsRegeneration = Prefs(struct {
PostureChecking bool PostureChecking bool
NetfilterKind string NetfilterKind string
DriveShares []*drive.Share DriveShares []*drive.Share
RelayServerPort *int RelayServerPort *uint16
RelayServerStaticEndpoints []netip.AddrPort RelayServerStaticEndpoints []netip.AddrPort
AllowSingleHosts marshalAsTrueInJSON AllowSingleHosts marshalAsTrueInJSON
Persist *persist.Persist Persist *persist.Persist

8
ipn/ipn_view.go
Unescape Escape View File

@ -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, // 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 // 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 used. A nil value signifies relay server functionality
// should be disabled. This field is currently experimental, and therefore // should be disabled.
// no guarantees are made about its current naming and functionality when func (v PrefsView) RelayServerPort() views.ValuePointer[uint16] {
// non-nil/enabled.
func (v PrefsView) RelayServerPort() views.ValuePointer[int] {
return views.ValuePointerOf(v.ж.RelayServerPort) return views.ValuePointerOf(v.ж.RelayServerPort)
} }
@ -506,7 +504,7 @@ var _PrefsViewNeedsRegeneration = Prefs(struct {
PostureChecking bool PostureChecking bool
NetfilterKind string NetfilterKind string
DriveShares []*drive.Share DriveShares []*drive.Share
RelayServerPort *int RelayServerPort *uint16
RelayServerStaticEndpoints []netip.AddrPort RelayServerStaticEndpoints []netip.AddrPort
AllowSingleHosts marshalAsTrueInJSON AllowSingleHosts marshalAsTrueInJSON
Persist *persist.Persist Persist *persist.Persist

10
ipn/prefs.go
Unescape Escape View File

@ -283,10 +283,8 @@ type Prefs struct {
// RelayServerPort is the UDP port number for the relay server to bind to, // 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 // 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 used. A nil value signifies relay server functionality
// should be disabled. This field is currently experimental, and therefore // should be disabled.
// no guarantees are made about its current naming and functionality when RelayServerPort *uint16 `json:",omitempty"`
// non-nil/enabled.
RelayServerPort *int `json:",omitempty"`
// RelayServerStaticEndpoints are static IP:port endpoints to advertise as // RelayServerStaticEndpoints are static IP:port endpoints to advertise as
// candidates for relay connections. Only relevant when RelayServerPort is // candidates for relay connections. Only relevant when RelayServerPort is
@ -694,7 +692,7 @@ func (p *Prefs) Equals(p2 *Prefs) bool {
p.PostureChecking == p2.PostureChecking && p.PostureChecking == p2.PostureChecking &&
slices.EqualFunc(p.DriveShares, p2.DriveShares, drive.SharesEqual) && slices.EqualFunc(p.DriveShares, p2.DriveShares, drive.SharesEqual) &&
p.NetfilterKind == p2.NetfilterKind && p.NetfilterKind == p2.NetfilterKind &&
compareIntPtrs(p.RelayServerPort, p2.RelayServerPort) && compareUint16Ptrs(p.RelayServerPort, p2.RelayServerPort) &&
slices.Equal(p.RelayServerStaticEndpoints, p2.RelayServerStaticEndpoints) slices.Equal(p.RelayServerStaticEndpoints, p2.RelayServerStaticEndpoints)
} }
@ -715,7 +713,7 @@ func (ap AppConnectorPrefs) Pretty() string {
return "" return ""
} }
func compareIntPtrs(a, b *int) bool { func compareUint16Ptrs(a, b *uint16) bool {
if (a == nil) != (b == nil) { if (a == nil) != (b == nil) {
return false return false
} }

2
ipn/prefs_test.go
Unescape Escape View File

@ -78,7 +78,7 @@ func TestPrefsEqual(t *testing.T) {
have, prefsHandles) have, prefsHandles)
} }
relayServerPort := func(port int) *int { relayServerPort := func(port uint16) *uint16 {
return &port return &port
} }
nets := func(strs ...string) (ns []netip.Prefix) { nets := func(strs ...string) (ns []netip.Prefix) {

6
net/udprelay/server.go
Unescape Escape View File

@ -309,7 +309,7 @@ func (e *serverEndpoint) isBound() bool {
// onlyStaticAddrPorts is true, then dynamic addr:port discovery will be // onlyStaticAddrPorts is true, then dynamic addr:port discovery will be
// disabled, and only addr:port's set via [Server.SetStaticAddrPorts] will be // disabled, and only addr:port's set via [Server.SetStaticAddrPorts] will be
// used. // 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{ s = &Server{
logf: logf, logf: logf,
disco: key.NewDisco(), disco: key.NewDisco(),
@ -526,9 +526,9 @@ func trySetUDPSocketOptions(pconn nettype.PacketConn, logf logger.Logf) {
// [magicsock.RebindingConn], which would also remove the need for // [magicsock.RebindingConn], which would also remove the need for
// [singlePacketConn], as [magicsock.RebindingConn] also handles fallback to // [singlePacketConn], as [magicsock.RebindingConn] also handles fallback to
// single packet syscall operations. // single packet syscall operations.
func (s *Server) listenOn(port int) error { func (s *Server) listenOn(port uint16) error {
for _, network := range []string{"udp4", "udp6"} { 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 err != nil {
if network == "udp4" { if network == "udp4" {
return err return err

5
net/udprelay/status/status.go
Unescape Escape View File

@ -14,8 +14,9 @@ import (
type ServerStatus struct { type ServerStatus struct {
// UDPPort is the UDP port number that the peer relay server forwards over, // UDPPort is the UDP port number that the peer relay server forwards over,
// as configured by the user with 'tailscale set --relay-server-port=<PORT>'. // as configured by the user with 'tailscale set --relay-server-port=<PORT>'.
// If the port has not been configured, UDPPort will be nil. // If the port has not been configured, UDPPort will be nil. A non-nil zero
UDPPort *int // value signifies the user has opted for a random unused port.
UDPPort *uint16
// Sessions is a slice of detailed status information about each peer // Sessions is a slice of detailed status information about each peer
// relay session that this node's peer relay server is involved with. It // relay session that this node's peer relay server is involved with. It
// may be empty. // may be empty.

Write Preview
Loading…
Cancel
Save