mirrors/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2026-05-04 11:51:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

wgengine/magicsock: remove pickPort, use port 0 to avoid TOCTOU race

Browse Source 
pickPort would bind a UDP socket on :0 to get a free port, close
the socket, then hope to rebind to the same port in NewConn. This
is a TOCTOU race that can cause flaky test failures when another
process grabs the port in between.

Instead, pass Port: 0 to NewConn and let the OS assign the port
atomically, then read back the assigned port via conn.LocalPort().

Fixes #19409

Change-Id: Ie44b599fb93c361e29a05f2171ad747c46f82b7a
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
This commit is contained in:
Avery Pennarun 2026-04-15 00:54:01 +00:00 committed by Brad Fitzpatrick
parent 6301a6ce4b
commit effbe67fe3
1 changed files with 12 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
wgengine/magicsock/magicsock_test.go
View File

@ -413,9 +413,11 @@ func TestNewConn(t *testing.T) {
stunAddr, stunCleanupFn := stuntest.Serve(t)
defer stunCleanupFn()
port := pickPort(t)
// Use port 0 to let the system assign a port, avoiding TOCTOU races
// from the previous pickPort approach which would close a socket and
// hope to rebind to the same port.
conn, err := NewConn(Options{
Port: port,
Port: 0,
DisablePortMapper: true,
EndpointsFunc: epFunc,
Logf: t.Logf,
@ -427,6 +429,13 @@ func TestNewConn(t *testing.T) {
t.Fatal(err)
}
defer conn.Close()
// Get the actual port that was assigned
port := conn.LocalPort()
if port == 0 {
t.Fatal("LocalPort returned 0")
}
conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String()))
conn.SetPrivateKey(key.NewNode())
@ -462,16 +471,6 @@ collectEndpoints:
}
}
func pickPort(t testing.TB) uint16 {
t.Helper()
conn, err := net.ListenPacket("udp4", "127.0.0.1:0")
if err != nil {
t.Fatal(err)
}
defer conn.Close()
return uint16(conn.LocalAddr().(*net.UDPAddr).Port)
}
func TestPickDERPFallback(t *testing.T) {
tstest.PanicOnLog()
tstest.ResourceCheck(t)
@ -1470,7 +1469,6 @@ func Test32bitAlignment(t *testing.T) {
// newTestConn returns a new Conn.
func newTestConn(t testing.TB) *Conn {
t.Helper()
port := pickPort(t)
bus := eventbustest.NewBus(t)
@ -1487,7 +1485,7 @@ func newTestConn(t testing.TB) *Conn {
Metrics: new(usermetric.Registry),
DisablePortMapper: true,
Logf: t.Logf,
Port: port,
Port: 0,
TestOnlyPacketListener: localhostListener{},
EndpointsFunc: func(eps []tailcfg.Endpoint) {
t.Logf("endpoints: %q", eps)