state: own request memory before storing nodes

A reused RegisterRequest.Hostinfo was written and stored while NodeStore readers cloned it.
This commit is contained in:
Kristoffer Dalby committed 2026-10-23 18:23:53 +02:00
1 parent 6d9fc8ac34
commit 51b420865e
4 files changed
+90 -4

No files matched your search

@@ -2201,3 +2201,52 @@ func TestPAKReregisterConcurrentAdminExpire(t *testing.T) {
require.True(t, n.IsExpired(), "iteration %d: the spent key revived an expired node", i)
}
}
func TestPreAuthKeyRegistrationOwnsHostinfo(t *testing.T) {
for _, name := range []string{"new", "reregister", "preserve NetInfo"} {
t.Run(name, func(t *testing.T) {
s := newRetagTestState(t)
user := s.CreateUserForTest("hostinfo-user")
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine().Public()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{
Hostname: "original",
Services: []tailcfg.Service{{Proto: "tcp", Port: 80}},
NetInfo: &tailcfg.NetInfo{PreferredDERP: 1},
},
}
if name != "new" {
_, _, err = s.HandleNodeFromPreAuthKey(regReq, machineKey)
require.NoError(t, err)
}
if name == "preserve NetInfo" {
regReq.Hostinfo.NetInfo = nil
}
original := regReq.Hostinfo.Clone()
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey)
require.NoError(t, err)
require.Equal(t, original, regReq.Hostinfo, "registration must not modify its request")
regReq.Hostinfo.Hostname = "changed"
regReq.Hostinfo.Services[0].Port = 443
if regReq.Hostinfo.NetInfo != nil {
regReq.Hostinfo.NetInfo.PreferredDERP = 2
}
stored, ok := s.GetNodeByID(node.ID())
require.True(t, ok)
require.Equal(t, "original", stored.Hostinfo().Hostname())
require.Equal(t, uint16(80), stored.Hostinfo().Services().At(0).Port)
require.Equal(t, tailcfg.DERPRegionID(1), stored.Hostinfo().NetInfo().PreferredDERP())
})
}
}
+37
View File
@@ -21,6 +21,43 @@ import (
"tailscale.com/types/opt"
)
func TestMapRequestOwnsRequestData(t *testing.T) {
_, s, nodeID := persistTestSetup(t)
t.Cleanup(func() { _ = s.Close() })
node, ok := s.GetNodeByID(nodeID)
require.True(t, ok)
req := tailcfg.MapRequest{
NodeKey: node.NodeKey(),
DiscoKey: node.DiscoKey(),
Endpoints: []netip.AddrPort{netip.MustParseAddrPort("192.0.2.1:41641")},
Hostinfo: &tailcfg.Hostinfo{
Hostname: "original",
Services: []tailcfg.Service{{Proto: "tcp", Port: 80}},
NetInfo: &tailcfg.NetInfo{
PreferredDERP: 1,
DERPLatency: map[string]float64{"1-v4": 0.1},
},
},
}
original := req.Hostinfo.Clone()
_, err := s.UpdateNodeFromMapRequest(nodeID, req)
require.NoError(t, err)
require.Equal(t, original, req.Hostinfo, "map update must not modify its request")
req.Hostinfo.Hostname = "changed"
req.Hostinfo.Services[0].Port = 443
req.Hostinfo.NetInfo.PreferredDERP = 2
req.Hostinfo.NetInfo.DERPLatency["1-v4"] = 0.2
req.Endpoints[0] = netip.MustParseAddrPort("192.0.2.2:41641")
stored, ok := s.GetNodeByID(nodeID)
require.True(t, ok)
require.Equal(t, original, stored.Hostinfo().AsStruct())
require.Equal(t, netip.MustParseAddrPort("192.0.2.1:41641"), stored.Endpoints().At(0))
}
func TestNetInfoFromMapRequest(t *testing.T) {
nodeID := types.NodeID(1)
+3 -3
View File
@@ -2776,8 +2776,7 @@ func (s *State) HandleNodeFromPreAuthKey(
}
// Ensure we have valid hostinfo
validHostinfo := cmp.Or(regReq.Hostinfo, &tailcfg.Hostinfo{})
validHostinfo.Hostname = hostname
validHostinfo := cmp.Or(regReq.Hostinfo.Clone(), &tailcfg.Hostinfo{})
log.Debug().
Caller().
@@ -3520,7 +3519,8 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
currentNode.ApplyPeerChange(&delta.peerChange)
if hostinfoToStore {
currentNode.Hostinfo = newHostinfo
// The snapshot outlives the request; deep-copy so its slices and NetInfo are never shared.
currentNode.Hostinfo = newHostinfo.Clone()
}
// Only a real hostname change may re-derive GivenName: it is peer
+1 -1
View File
@@ -661,7 +661,7 @@ func (node *Node) PeerChangeFromMapRequest(req tailcfg.MapRequest) tailcfg.PeerC
// Compare endpoints using order-independent comparison
if EndpointsChanged(node.Endpoints, req.Endpoints) {
ret.Endpoints = req.Endpoints
ret.Endpoints = slices.Clone(req.Endpoints)
}
now := time.Now()